String in ANSI C?

Question

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

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdarg.h>
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 < size)
 return p;
 // failed: have to try again, alloc more mem. 
 if(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("---------------------------------------------------\n");
 printf("\n");
}

In this code I have 3 functions to handle the string:

1. InitStr - to initial string
2. AddStr - to add string
3. JoinStr -to Join string

The code works fine, however, I am not sure if this is a decent way of handling a string since I am using pointers.

• wildplasser: for a beginner, this is reasonable source code. the `char *InitStr(char *ReturnStr){ ReturnStr = NULL; ...} however, makes no sense. Why give an argument to a function, knowing that the first thing the function does is setting it to NULL ?
• sarnold: Hopefully useful: gratisoft.us/todd/papers/strlcpy.html
• Jim Balter: InitStr will work but consider char* EmptyStr(void){ return strdup(""); }

Answer 1

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);
}

• G7fya: thank you for reviewing my code. your suggestions are greatly appreciated. However, I have 2 more thing to ask. 1. where/why use char * increaseBuffer() since i have char *AddStr(char *StrObj1,char *StrObj2)? 2. should i use free(MyString) before exit the program?
• David M. Syzdek: @FlanAlflani 1. increaseBuffer() is just a hypothetical example and is not used within your program. 2. Yes, you are correct. Any memory you allocate, must also free before you exit the program or re-use the pointer.
• Jens Gustedt: Just as a nitpick the correct prototype would be char *InitStr(void). With empty () at the call side this is only "known" as a function that receives any kind of argument, so some error checking would be off.
• G7fya: @David I keep getting *** glibc detected *** ./myprogram: munmap_chunk(): invalid pointer: 0x0000000002588ca0 *** when try to use JoinStr

Answer 2

This question might be more appropriate for programmers or codereview, but, this is what a code review of mine, probably with at least one controversial statement inside somewhere, would go like. Hopefully you will find some advice in here that answers your questions.

1) Make sure you want to be using C and not C++ (or Java, Perl...) for whatever you're doing. If you can avoid doing strings in C in real life code, always. Use C++ strings instead. And if you want a sturdy C library to do this in, consider glib (or writing C++ wrappers).

2) If you want to write a full ADT (abstract data type) for strings, follow the usual C paradigm: You should have functions mystring_init, mystring_{whateveroperations}, and mystring_cleanup. The init will typically have a malloc and if so cleanup will definitely have free. The reallocs I think (not sure) are only safe provided malloc happens first and free last, which without an init that mallocs would be bad.

3) But in real life I think an ADT for strings, which experienced C developers can program in pretty fluently, will make the code less readable, not more.

4) Always work on the stack when possible. char str[STR_MAX_SIZE] is preferable to dynamic strings when possible, and it's possible more than most developers think. A few wasted bytes from safely overestimating is worth avoiding a crash or leak from dynamic memory usage. And your function JoinStr from its signature looks like it will accept a stack variable as its first argument, then bam! realloc. If you're going to do something sneaky like this, it pretty much has to be with an ADT you wrote via a pattern similar to (2).

5) Along those lines, the usual way to pass a string to a function is myoperation(char* str, size_t size); so the caller is responsible for memory management - and in particular gets to choose whether it's stack or heap - and the inside of the function just respects those parameters passed. If you braek this I strongly recommend the ADT pattern in (2).

Answer 3

char *InitStr(char *ReturnStr){
 ReturnStr = NULL; 

This line throws away whatever the user passed in as the parameter

 ReturnStr = realloc( ReturnStr, strlen("") +1 );

This throws away what the previous line does. It should also be pretty obvious the strlen("") is, so why calculate it?

 strcpy( ReturnStr , "");

This line only ends up doing the same as ReturnStr[0] = '0円'

 return ReturnStr;
}

The whole function should be written as strdup("").

You assume that a negative return value means not enough memory was provided. However, its possible that any of a number of things could have gone wrong. If something else wrong, bad format string for example, this function will sit there exhausting your memory trying to fill out the string.