C segmentation fault using malloc'ed strings

Question

Essentially, I have a structure for a linked list in my code,

struct node{
  char* val;
  struct node *next;
};

Then later I try to do some things to it...

...
addr = malloc(sizeof(struct node));
addr->val = malloc(72);
addr->val = "";
snprintf(addr->val, 1000, "%s", ident);
...

... This gives me a segmentation fault at the snprintf. Valgrind says the following

Process terminating with default action of signal 11 (SIGSEGV)
==10724==  Bad permissions for mapped region at address 0x40566B
==10724==    at 0x4EB0A32: vsnprintf (vsnprintf.c:112)
==10724==    by 0x4E8F931: snprintf (snprintf.c:33)
==10724==    by 0x4016CC: id (Analyzer.c:267)
...

I am fairly new to C as opposed to C++, but I thought that calling malloc on the char* should make it valid, especially since I can initialize and print it, so I don't understand why the snprintf wouldn't work. I also had my program print out the addresses of both variables, and the address valgrind complains about is indeed from addr->val.

I also tried using strcpy instead of snprintf but had the same result.

Thanks.

score 2 · Accepted Answer · edited Nov 26 '17 at 04:38

addr->val = malloc(72);

This line dynamically allocates 72 bytes and assigns the address of that memory region to addr->val.

addr->val = "";

This then sets addr->val to point to the address of a string constant, discarding the address of the malloced area of memory that it previously contained, causing a memory leak.

When you then try to use snprintf, you're attempting to write to a string literal. Since these are typically stored in a read-only section of memory, attempting to do so results in a core dump.

There's no need for addr->val = "";. It throws away allocated memory; it doesn't set that allocated memory to an empty string, which is probably what you thought it would do. Even if it did, it's useless anyway because snprintf will overwrite anything that might be there.

score 0 · Answer 2 · answered Nov 26 '17 at 02:19

The code

addr->val = malloc(72);
addr->val = "";             <====

overwrites val pointer with "", address of a static area, made of 1 character (of value 0). Remove this line.

And

snprintf(addr->val, 1000, "%s", ident);

Accept a length of 1000 while you only allocated 72 characters.

snprintf(addr->val, 72, "%s", ident);

is better.

score 0 · Answer 3 · answered Nov 26 '17 at 02:23

addr->val = malloc(72);
addr->val = "";

The second line re-assignes addr->val to an address in read-only section (where string literals lie), and discards the address of allocation from malloc, which can lead to a potential memory leak.

I know you want to clear it. To assign strings, you should use strcpy()

strcpy(addr->val, "");

But since you want to empty it, it's easiest to set the first character to zero:

addr->val[0] = '\0';

Besides, you're trying to do a potentially harmful job:

snprintf(addr->val, 1000, "%s", ident);

You intended to allocate 72 bytes, but why us the second parameter going up to 1k? Change it to a safer number:

snprintf(addr->val, 72, "%s", ident);

Everything should be fine by then.

C segmentation fault using malloc'ed strings

3 Answers3