Dynamic memory allocation with strings

[bops]

Hey all,
I was just wondering, is it safe to do something like this?

char *msg;
sprintf(msg,"Forename: %s", fname); //where fname has been previously defined as char fname[ ] = "Jonathon";

My main query is around the allocation of the memory for msg. I know there are memory allocation functions like malloc and such that can safely allocate memory for you. But what happens if one does not know for example how long fname is, since it could contain anything, as well as whatever else we want to put into message, in this case "Forename: ". In the above example I would assume the best thing to do would be to use malloc to allocate enough space to contain everything. Basically, what if it is impossible to calculate the amount of space required to hold everything in the array?.. I just wanted to know whats the best way to work with strings and memory allocation in C?

Cheers,
Jonathon.

[ssharish2005]

YES, you need to allocate memory for msg before you copy the string on to msg. Failing to do that will lead to a Segmentation fault.

ssharish

[bops]

Yeah, I assumed so. But what is the best way to go about doing this? What kind of practices do you use when dealing with such things?

[Narue]

>I was just wondering, is it safe to do something like this?
No, an uninitialized pointer doesn't point to infinite usable memory.

>Basically, what if it is impossible to calculate the amount
>of space required to hold everything in the array?
Then you've done something wrong. You should always be able to calculate the size of your memory blocks to within a reasonable margin of waste, or you should be able to incrementally resize your blocks until all of the data is stored.

In your specific case, it's possible to calculate the necessary size for msg by taking the length of fname and adding it to the length of the format string. Alternatively you could use something like snprintf, which is generally better at giving you more information.

For things like streamed input where you simply cannot calculate the final length ahead of time, you're stuck with using a fixed-size block to read chunks and growing the result buffer until there aren't any more chunks.

[bops]

Ok, thanks a lot.

Actually, can you use sprintf like this, and is it safe?

int len;
len = sprintf(NULL,"This is my forename: %s",fname);

And then use len to allocate the memory for msg?

[Tilir]

Ok, thanks a lot.

Actually, can you use sprintf like this, and is it safe?

int len;
len = sprintf(NULL,"This is my forename: %s",fname);

And then use len to allocate the memory for msg?

No at all. Right way is to use strlen, and then dynamically allocate buffer like this:

const char formatstr[] = "This is my forename: %s";
char name[] = "Erick, the Blood Axe"; // Try you name instead
int totalLen = strlen(formatstr) + strlen(name);
char* buffer = (char*) malloc (sizeof(char) * totalLen); // allocate memory
sprintf(buffer, formatstr, name); // no AV here
// use buffer as you wish
free(buffer); buffer = NULL; // free memory

[Prabakar]

>> int totalLen = strlen(formatstr) + strlen(name);
"%s" will be counted. The totalLen should be 41 but obtained totalLen is 43.
And,
>> len = sprintf(NULL,"This is my forename: %s",fname);
seems to work perfectly!!!
Is there an assurance that the string will not be written if NULL is passed? I believe not. Is this undefined behavior?

[Aia]

int totalLen = strlen(formatstr) + strlen(name);

I would try to avoid this kind of statements since the order of evaluation of the two functions is not defined. If one function affects the result of the other, then side effects will result.

char* buffer = (char*) malloc (sizeof(char) * totalLen); // allocate memory

Casting the return of malloc is not a good practice neither.
sizeof(char) is redundant since char is a byte. It would be better to sizeof(*buffer) in that way if the pointer type gets changed still works. Forgot to add +1 in malloc, for the '\0'.

free(buffer);

freeing memory that it might not be allocated is a no no. Check that malloc was succesful.

[Salem]

> >> len = sprintf(NULL,"This is my forename: %s",fname);
> seems to work perfectly!!!
> Is there an assurance that the string will not be written if NULL is passed?
> I believe not. Is this undefined behavior?
It certainly is undefined behaviour.

However, if you have a C99 compiler, then you have snprintf() as well. It is legal to pass NULL to this in order to measure the length of a formatted string.

[ArkM]

As far as I know it's scprintf(const char*,...) in C99 counts the length of a formatted string (and it's not legal to pass null pointer to sprintf family functions).
May be, I'm wrong?..

[Salem]

There is no scprintf in ANSI.
It's a Microsoft thing to solve the same problem.

[Tilir]

len = sprintf(NULL,"This is my forename: %s",fname);
seems to work perfectly!!!
Is there an assurance that the string will not be written if NULL is passed? I believe not. Is this undefined behavior?

According to the C99 Standard, 7.19.6.6.3, "The sprintf function returns the number of characters written in the array, not counting the terminating null character, or a neg ative value if an encoding error occurred".

So in the case of NULL in the first argument, sprintf can not write any characters in the array and so MUST return -1 in C99-compliant compilers. If you are lucky with one compiler (such as microsoft), it really means nothing. IMHO anyone should avoid to use non-portable hacks.

[Tilir]

int totalLen = strlen(formatstr) + strlen(name);
I would try to avoid this kind of statements since the order of evaluation of the two functions is not defined. If one function affects the result of the other, then side effects will result.

There are no side effects in strlen, except may be errno.

char* buffer = (char*) malloc (sizeof(char) * totalLen); // allocate memory
Casting the return of malloc is not a good practice neither.
sizeof(char) is redundant since char is a byte. It would be better to sizeof(*buffer) in that way if the pointer type gets changed still works. Forgot to add +1 in malloc, for the '\0'.

Yes it's right. sizeof(*buffer) * (len + 1) will be correct.

free(buffer);
freeing memory that it might not be allocated is a no no. Check that malloc was succesful.

Of course this must be rewritten as

if (buffer){
  free(buffer);
  buffer = NULL;
}

But in one-liner I just skipped error-handling for clarity.

Thank you for your corrections.

[Salem]

Testing buffer before free achieves nothing useful.

Description
2 The free function causes the space pointed to by ptr to be deallocated, that is, made
available for further allocation. If ptr is a null pointer, no action occurs.

If buffer is NULL, then freeing it does nothing
If buffer is not NULL, but a garbage pointer, then the test doesn't save you from disaster.

But setting buffer to NULL afterwards is definitely a good idea, as it will easily detect any inadvertant "use after free" mistakes.

[Tilir]

Testing buffer before free achieves nothing useful.

If buffer is NULL, then freeing it does nothing
If buffer is not NULL, but a garbage pointer, then the test doesn't save you from disaster.

But setting buffer to NULL afterwards is definitely a good idea, as it will easily detect any inadvertant "use after free" mistakes.

Yes, you are right, standard 7.20.3.2. So final version will be

const char formatstr[] = "This is my forename: %s";
  char name[] = "Erick, the Blood Axe"; // Try you name instead
  size_t formatstrlen = strlen(formatstr);
  size_t totalLen = formatstrlen + strlen(name);
  char* buffer = (char*) malloc (sizeof(*buffer) * (totalLen + 1)); // allocate memory
  if (buffer){
    sprintf(buffer, formatstr, name); // no AV here
    // use buffer as you wish
  }
  free(buffer); buffer = NULL; // free memory

The main disadvantage of this solution is excessive memory allocation for format specifiers -- "%s" and so on, that is not in result string, but it is rather small amount of memory.

2Aia:
Please explain how to rewrite this code without casting of the return value of malloc. I think it simply wouldn't compile without explicit typecast to (char*).

[Aia]

2Aia:
Please explain how to rewrite this code without casting of the return value of malloc. I think it simply wouldn't compile without explicit typecast to (char*).

Include the header file stdlib.h where malloc is prototype correctly.
Some more on it.

[ssharish2005]

>simply wouldn't compile without explicit typecast to (char*).
Which compiler are you using??

ssharish

[Salem]

> I think it simply wouldn't compile without explicit typecast to (char*).
Only in two cases, neither of them good.
1) you're using a pre-ansi compiler, where malloc returns the previous 'any' pointer type of char*. Since ANSI first came out in 1989 (that's 20 years ago!), why are you still using such a fossil?
2) you're using a C++ compiler to compile C code. In which case, learn to use the correct tool for the job. Don't mung your programs into polyglots just to make them compile as C or C++. All that leads to is code which is both bad C and bad C++.

You don't need to cast malloc in a valid ANSI C program, compiled with an ANSI C compiler.

[Raptor007]

Sorry to bump an ancient thread, but I actually found a great solution to this problem that hadn't been mentioned:

char *str = NULL;
asprintf( &str, "My cat %s has $%.2f in her bank account.", name, money );

This function will perform the allocation (with malloc) and then use that allocated space as the sprintf output buffer. You should free the memory when you're finished with it. It returns a negative value for any errors, so you may want to check for that.

I believe this is a GNU C extension, so it may not be supported on every platform.