Hi,
Is it the correct way of allocating memory to a char*.
char* sides ="5";
char* tempSides;
tempSides = (char*)malloc(strlen(inSides) * sizeof(char));
Hi,
Is it the correct way of allocating memory to a char*.
char* sides ="5";
char* tempSides;
tempSides = (char*)malloc(strlen(inSides) * sizeof(char));
Almost. Strings are NULL terminated, so you probably want to allocate an extra byte to store the NULL byte. That is, even though sides
is 1 character long, it really is 2 bytes: {5
,'\0'
}.
So it would be:
tempSides = (char *)malloc((strlen(sides)+1)*sizeof(char));
and if you wanna copy it in:
strcpy(tempSides, sides);
There's a problem with that. tempSides will point to an uninitialized block of memory of size 1. If you intend to copy the sides string into tempSides, then you will need to allocate a size one byte longer, in order to hold the zero terminator for the string. The value returned by strlen() does not include the zero terminator at the end of the string.
No, not really. As others have already noted, you need to allocate space for the NUL terminator.
In addition, you generally should not cast the return from malloc
. It can cover up a bug where you've forgotten to #include
the correct header. Multiplying by sizeof(char)
is also pointless, since the standards (both C and C++) define sizeof(char)
to always be 1.
Finally, every call to malloc
should include a test of the result. I'd wrap the whole thing up into a function:
char *dupe_string(char const *string) {
char *temp;
if (NULL!=(temp=malloc(strlen(string)+1)))
strcpy(temp, string);
return temp;
}
As has been pointed out, you missed allocating space for the terminating NUL chararacter. But I also wanted to point out a couple of other things that can make your code more concise.
By definition, sizeof(char)
is always 1, so you can shorten your allocation line to:
tempSides = (char*)malloc(strlen(inSides) + 1);
Another thing is that this looks like you are doing to duplicate the string. There is a built in function that does that for you:
tempSides = strdup(inSides);
This handles getting the length, allocating the correct number of bytes and copying the data.
Note that:
char *
;So that would be:
char *tempSides = malloc(strlen(inSides) + 1);
Still, if you want to duplicate the contents of inSides
, you can use strdup
, e.g.:
char *tempSides = strdup(inSides);
if (tempSides != NULL) {
// do whatever you want...
free(tempSides);
}
Multiplying the element count by sizeof(char)
is a matter of personal preference, since sizeof(char)
is always 1. However, if you do this for consistency, better use the recipient pointer type to determine the element size, instead of specifying type explicitly. And don't cast the result of malloc
tempSides = malloc(strlen(inSides) * sizeof *tempSides);
Of course, when working with zero-terminated strings you have to remember to allocate extra space for the terminating zero character. There's no way to say whether it is your intent to make tempSides
a zero-terminated string in this case, so I can't say whether you need it.