HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

String in ANSI C?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
ansistringstackoverflow

Problem

I'm trying declare a string in ANSI C and am not sure of the best way to do it.

```
#include
#include
#include
#include

char InitStr(char ReturnStr){
ReturnStr = NULL;
ReturnStr = realloc( ReturnStr, strlen("") +1 );
strcpy( ReturnStr , "");
return ReturnStr;
}
char AddStr(char StrObj1,char *StrObj2){
StrObj1 = realloc(StrObj1,(strlen(StrObj1) + strlen(StrObj2)+1));
strcat(StrObj1, StrObj2);
return StrObj1 ;
}
char JoinStr(const char fmt, ...) {
char *p = NULL;
size_t size = 30;
int n = 0;
va_list ap;

if((p = malloc(size)) == NULL)
return NULL;

while(1) {
va_start(ap, fmt);
n = vsnprintf(p, size, fmt, ap);
va_end(ap);

if(n > -1 && n -1) // glibc 2.1
size = n + 1;
else //* glibc 2.0
size *= 2; // twice the old size

if((p = realloc (p, size)) == NULL)
return NULL;
}
}
main(){
printf("\n");

char *MyLocalString = InitStr(MyLocalString);
printf("InitStr: %s\n",MyLocalString);
printf("---------------------------------------------------\n");

AddStr(MyLocalString ,"Hello String!");
printf("1. AddStr: %s\n",MyLocalString);
printf("---------------------------------------------------\n");

AddStr(MyLocalString ,"\n\tAdd more string 1");
printf("2. AddStr: %s\n",MyLocalString);
printf("---------------------------------------------------\n");

AddStr(MyLocalString ,"\n\tAdd more string 2");
printf("3. AddStr: %s\n",MyLocalString);
printf("---------------------------------------------------\n");

//MyLocalString = AddStr(MyLocalString ,"\n Add more string");
MyLocalString = AddStr(MyLocalString ,JoinStr("%s%s%s", "\n\tString3", "\n\tString2", "\n\tString3"));
printf("4. JoinStr: %s\n",MyLocalString);
printf("--------------------------------------------

Solution

I have a few suggestions.

First you are not verifying that realloc() properly allocated memory. When using realloc() you should use two pointers so you can properly check for a successful allocation:

char * increaseBuffer(char * buff, size_t newBuffSize)
{
   void * ptr;

   // adjust buffer size
   ptr = realloc(buff, newBuffSize);

   // verify allocation was successful
   if (ptr == NULL)
   {
      perror("realloc()"); // prints error message to Stderr
      return(buff);
   };

   // adjust buff to refer to new memory allocation
   buff = ptr;

   /* use buffer for something */

   return(buff);
};


Since your InitStr() function is only providing an initial allocation, you can simplify it to:

char *InitStr(){
    char * ReturnStr;

    // allocate new buffer
    ReturnStr = malloc(1);

    // verify allocation was successful before using buffer
    if (ReturnStr != NULL)
        ReturnStr[0] = '\0';

    // return buffer
    return(ReturnStr);
}


This could also be condensed a little more:

char *InitStr(){
    char * ReturnStr;

    if ((ReturnStr = malloc(1)) != NULL)
        ReturnStr[0] = '\0';

    return(ReturnStr);
}


Here is another example of checking your allocation using your AddStr() function:

char *AddStr(char *StrObj1,char *StrObj2){
    void * ptr;
    ptr = realloc(StrObj1,(strlen(StrObj1) + strlen(StrObj2) + 1));
    if (ptr == NULL)
        return(StrObj1);
    StrObj1 = ptr;
    strcat(StrObj1, StrObj2);
    return(StrObj1);
}


You can slightly improve the runtime efficiency of this function be reducing the number of times you need to find the terminating NULL character in your strings:

char *AddStr(char *StrObj1,char *StrObj2){
    void * ptr;
    size_t len1;
    size_t len2;

    // determine length of strings
    len1 = strlen(StrObj1);
    len2 = strlen(StrObj2);

    // increase size of buffer
    if ((ptr = realloc(StrObj1, (len1 + len2 + 1))) == NULL)
        return(StrObj1);
    StrObj1 = ptr;

    // this passes a pointer which references the terminating '\0' of
    // StrObj1. This makes the string appear to be empty to strcat() which
    // in turn means that it does not compare each character of the string
    // trying to find the end. This is a minor performance increase, however if
    // running lots of string operations in a high iteration program, the combined
    // affect could be substantial. 
    strcat(&StrObj1[len1], StrObj2);

    return(StrObj1);
}


You can complete your last function without the while loop:

char *JoinStr(const char *fmt, ...)
{

   int len;
   char * str;
   void * ptr;
   va_list ap;

   // start with a small allocation to pass to vsnprintf()
   if ((str = malloc(2)) == NULL)
      return(NULL);
   str[0] = '\0';

   // run vsnprintf to determine required length of string
   va_start(ap, fmt);
      len = vsnprintf(str, 2, fmt, ap);
   va_end(ap);

   if (len < 2)
      return(str);

   // allocate enound space for entire formatted string
   len++;
   if ((ptr = realloc(str, ((size_t)len))) == NULL)
   {
      free(str);
      return(NULL);
   };

   // format string
   va_start(ap, fmt);
   vsnprintf(str, len, fmt, ap);
   va_end(ap);

   return(str);
}

Code Snippets

char * increaseBuffer(char * buff, size_t newBuffSize)
{
   void * ptr;

   // adjust buffer size
   ptr = realloc(buff, newBuffSize);

   // verify allocation was successful
   if (ptr == NULL)
   {
      perror("realloc()"); // prints error message to Stderr
      return(buff);
   };

   // adjust buff to refer to new memory allocation
   buff = ptr;

   /* use buffer for something */

   return(buff);
};
char *InitStr(){
    char * ReturnStr;

    // allocate new buffer
    ReturnStr = malloc(1);

    // verify allocation was successful before using buffer
    if (ReturnStr != NULL)
        ReturnStr[0] = '\0';

    // return buffer
    return(ReturnStr);
}
char *InitStr(){
    char * ReturnStr;

    if ((ReturnStr = malloc(1)) != NULL)
        ReturnStr[0] = '\0';

    return(ReturnStr);
}
char *AddStr(char *StrObj1,char *StrObj2){
    void * ptr;
    ptr = realloc(StrObj1,(strlen(StrObj1) + strlen(StrObj2) + 1));
    if (ptr == NULL)
        return(StrObj1);
    StrObj1 = ptr;
    strcat(StrObj1, StrObj2);
    return(StrObj1);
}
char *AddStr(char *StrObj1,char *StrObj2){
    void * ptr;
    size_t len1;
    size_t len2;

    // determine length of strings
    len1 = strlen(StrObj1);
    len2 = strlen(StrObj2);

    // increase size of buffer
    if ((ptr = realloc(StrObj1, (len1 + len2 + 1))) == NULL)
        return(StrObj1);
    StrObj1 = ptr;

    // this passes a pointer which references the terminating '\0' of
    // StrObj1. This makes the string appear to be empty to strcat() which
    // in turn means that it does not compare each character of the string
    // trying to find the end. This is a minor performance increase, however if
    // running lots of string operations in a high iteration program, the combined
    // affect could be substantial. 
    strcat(&StrObj1[len1], StrObj2);

    return(StrObj1);
}

Context

StackExchange Code Review Q#12428, answer score: 3

Revisions (0)

No revisions yet.