3

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

1
  • 1
    you do not need to cast malloc, its return type is void* and malloc(sizeof(char) * 5) is the same as malloc(sizeof(1) * 5), anyway you need at least 6 and not 5. Commented Aug 19, 2015 at 15:55

1 Answer 1

3

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.

Sign up to request clarification or add additional context in comments.

1 Comment

@Michi I don't normally. It was a copy-paste error from the question.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.