user5244097
user5244097

Reputation: 31

Error when using a structure with an array of chars in C

I have a struct that contains an array of chars and another struct that has this struct. I passed it by reference to a function that initializes the array. Then, I call a function to print and free the array.

Please help me, what is wrong with this program?

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

typedef struct {
  char *x;
  double y;
} A;

typedef struct {
  A a;
  int w;
} B;

void f(B *b) {
  int i;

  b->w = 5;
  b->a.x = (char *) malloc(sizeof(char) * 5);
  printf("addres in f %p: \n", b->a.x);

  for(i = 0; i < 5; i++)
    b->a.x[i] = 'a';

  b->a.y = 20;
}

void p(B *b) {
  int i;

  printf("w: %d\n", b->w);
  printf("addres in p %p: \n", b->a.x);
  printf("x: %s\n", b->a.x);
  printf("y: %f\n", b->a.y);
  free(b->a.x);
}

int main(int argc, char **argv) {
  B b;

  f(&b);
  p(&b);

  return 0;
}

When I run with valgrind, it occurs the following:

==28053== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 6 from 2)
==28053== 
==28053== 1 errors in context 1 of 1:
==28053== Invalid read of size 1
==28053==    at 0x3A03E489D7: vfprintf (in /lib64/libc-2.10.1.so)
==28053==    by 0x3A03E4FB49: printf (in /lib64/libc-2.10.1.so)
==28053==    by 0x400633: p (test_struct.c:33)
==28053==    by 0x400686: main (test_struct.c:43)
==28053==  Address 0x4c20035 is 0 bytes after a block of size 5 alloc'd
==28053==    at 0x4A0763E: malloc (vg_replace_malloc.c:207)
==28053==    by 0x400574: f (test_struct.c:19)
==28053==    by 0x40067A: main (test_struct.c:42)

and the output:

addres in f 0x16b4010: 
w: 5
addres in p 0x16b4010: 
x: aaaaa
y: 20.000000

Thanks, hg

Upvotes: 3

Views: 41

Answers (1)

IKavanagh
IKavanagh

Reputation: 6187

You haven't null-terminated your 'string' b->a.x so printf("x: %s\n", b->a.x); reads past the end of the allocated memory which causes the error you are getting in valgrind.

You can fix it by changing

b->a.x = (char *) malloc(sizeof(char) * 5);
  printf("addres in f %p: \n", b->a.x);

  for(i = 0; i < 5; i++)
    b->a.x[i] = 'a';

to

b->a.x = malloc(6 * sizeof b->a.x);
printf("address in f %p: \n", b->a.x);

for(i = 0; i < 5; i++)
    b->a.x[i] = 'a';
b->a.x[i] = '\0';

Here I've increased the size of the dynamically allocated block of memory to 6 and I've explicitly null-terminated the 'string' with b->a.x[i] = '\0';. \0 is used to null-terminate strings in C.


Note: As pointed out by @Michi in the comments. In C there is no need to cast the result of malloc().

I've also rewritten sizeof(char) to sizeof b->a.x as it is better for portability in case you change the type of b->a.x.

Upvotes: 3

Related Questions