13

If I would write:

char *a=malloc(sizeof(char)*4);
a="abc";
char *b="abc";

do I need to free this memory, or is it done by my system?

Andna
  • 6,539
  • 13
  • 71
  • 120
  • One additional comment is that I believe you don't need sizeof(char) since in all the implementations I have seen so far that's always 1 byte. So malloc(4); would suffice – Lefteris Apr 08 '12 at 13:46
  • 2
    @Lefteris actually the above implementation is good. Although most systems use one byte chars, the implementation above leaves it clear what is being allocated, and if the code is changed to use unichars, which are usually two bytes, then it will be easier to change. – ThomasW Apr 08 '12 at 14:02
  • @ThomasW I am afraid I am not sure what exactly a unichar is but I supposed it has to do with unicode. A quick google search suggests objective-c which is another language :P But any unicode characters typically are not called just char. Take wide characters wchar_t for example. Could be wrong though, but this is what I have seen so far with the implementations I have dealt with – Lefteris Apr 08 '12 at 14:09
  • Yes, a unichar is a Unicode character, but the concept is not specific to objective-C. In general is is not a good idea to make assumptions about the size of data structures. – ThomasW Apr 08 '12 at 14:26

6 Answers6

18

In your situation you won't have any way to free the dynamic allocated memory because you are losing the reference to it.

Try this out:

#include <stdio.h>
#include <stdlib.h>

int main()
{
  char *a=(char*)malloc(sizeof(char)*4);
  printf("Before: %p\n",a);
  a = "abc";
  printf("After: %p\n",a);
  free(a);
  char *b = "abc";

  return 0;
}

You will obtain

Before: 0x100100080
After: 0x100000f50

You will see that the two pointers are different. This because the string literal "abc" is placed into data sector of the binary files and when you do

a = "abc"

you are changing the pointer of a to point to the constant literal string "abc" and you are losing the previously allocated memory. Calling free on a is not correct anymore, just because it won't point to a valid dynamically allocated address anymore. To preserve the pointer and be able to free it you should copy the string with

strncpy(a, "abc", 4)

This will effectively copy characters from the literal to the dynamically allocated method, preserving the original pointer.

Jack
  • 131,802
  • 30
  • 241
  • 343
  • 4
    strcpy is the root of many buffer overflows. Use strncpy instead. – rampion Apr 08 '12 at 13:51
  • You are right, I was just considering this specific case, let me edit it – Jack Apr 08 '12 at 13:52
  • @rampion Just out of curiosity why do you say this? Because strcpy copies strlen(source) bytes which might not always be checked against the buffer size of the destination? Or I understand wrong? – Lefteris Apr 08 '12 at 13:53
  • 1
    @Lefteris It is not checked at all, `strcpy` will copy `strlen(source)` without caring about destination size. If source length > dest length then it will write outside bounds. – Jack Apr 08 '12 at 13:54
  • 1
    @Andna of course if you use `strncpy` you will have to release the memory with `free(a)` at the end of the code. – Jack Apr 08 '12 at 13:56
  • @Jack yeah this is what I wanted to say. By be checked I meant, by you(the programmer). Strncpy since it requires that extra variable there makes you think, "hey I gotta check stuff here, nothing is automagic" :) – Lefteris Apr 08 '12 at 13:58
6

You've got a memory leak here. When you set a="abc", you're not filling the memory you just allocated, you're reassigning the pointer to point to the static string "abc". b points to the same static string.

What you want instead is strncpy(a, "abc", 4), which will copy the contents of "abc" into the memory you allocated (which a points to).

Then you would need to free it when finished.

rampion
  • 87,131
  • 49
  • 199
  • 315
3

You cannot assign string in this way with C

a = "abc"

However if you use malloc then you have to use free, like this

free(a);

But pay attention if you use free(a) in your example you get an error. Because after the malloc you change the pointer a value to the static string "abc"; So the next free(a) try to free a static data. And you get the error.

dash1e
  • 7,677
  • 1
  • 30
  • 35
3

Simple answer yes,no. Also your code is buggy.

Concrete answer:

char *a=malloc(sizeof(char)*4);

You allocate memory so you should free it.

a="abc";

This assigns a pointer to a constant string to your char* a, by doing so you loose the pointer to the memory allocated in the first line, you should never free constant strings.

Use strcpy(a,"abc"); instead of a="abc"; to move the string into your allocated memory.

josefx
  • 15,506
  • 6
  • 38
  • 63
0

Yes, it will cause a memory leak. The system could not handle the case.

Brian
  • 14,610
  • 7
  • 35
  • 43
Kevin
  • 11
  • 2
-1

yes, you need to free the memory returned by malloc.

paquetp
  • 1,639
  • 11
  • 19