Writing struct + write(buf) points to uninitialised byte(s)

Question

I am allocating space for the struct node in the variable n0. I save this struct to file using fwrite, but when I run valgrind I get this error. My code is below, could you help me please?

==1412== Syscall param write(buf) points to uninitialised byte(s)
==1412==    at 0x4F22870: __write_nocancel (syscall-template.S:81)
==1412==    by 0x4EB0002: _IO_file_write@@GLIBC_2.2.5 (fileops.c:1261)
==1412==    by 0x4EB14DB: _IO_do_write@@GLIBC_2.2.5 (fileops.c:538)
==1412==    by 0x4EB0D5F: _IO_file_close_it@@GLIBC_2.2.5 (fileops.c:165)
==1412==    by 0x4EA4B0F: fclose@@GLIBC_2.2.5 (iofclose.c:59)
==1412==    by 0x400793: main (in /home/grados-sanchez/git/merkle-codigos-C/test_file)
==1412==  Address 0x402500c is not stack'd, malloc'd or (recently) free'd
==1412==  Uninitialised value was created by a stack allocation
==1412==    at 0x40073F: main (in /home/grados-sanchez/git/merkle-codigos-C/test_file)

typedef struct {
    unsigned char * ustr;
    int height;
}node;

void node_init(node * n, int r) {

    int i;
    n->ustr = malloc((r + 1) * sizeof(unsigned char));
    for (i = 0; i < r; i++) {
        (n->ustr)[i] = random() & 0xff;
    }
    (n->ustr)[r] = 0;
    n->height = -1;
}
void node_destroy(node * n) {
    free(n->ustr);
    n->height = -1;
}

int main() {
    FILE* file_ptr = fopen("file1", "w+");
    node n0;
    node_init(&n0,2);
    fwrite(&n0, sizeof(node), 1, file_ptr);
    fclose(file_ptr);
    node_destroy(&n0);
    return 0;
}

Answer 1

This happens because the compiler is padding your struct and you're writing the extra padding bytes but not initializing them. You can see this by first running your program, and then examining what comes out:

$ od -x file1
0000000 92c0 04c2 0000 0000 ffff ffff 0000 0000
0000020

The first 8 bytes (92c0 04c2 0000 0000) are your pointer value, ustr (note that it's writing the value of the pointer itself, not what it points to, which may well not be what you intended, but that's a separate issue).

The next four bytes (ffff ffff) are your int height which you set to -1.

And then there are four more bytes set to 0. These are the padding inserted by the compiler which you did not initialize. You can prove to yourself that this is the case by modifying your program slightly to make the padding explicit:

typedef struct {
    unsigned char * ustr;
    int height;
    int pad;
}node;

void node_init(node * n, int r) {

    int i;
    n->ustr = malloc((r + 1) * sizeof(unsigned char));
    for (i = 0; i < r; i++) {
        (n->ustr)[i] = random() & 0xff;
    }
    (n->ustr)[r] = 0;
    n->height = -1;
    n->pad = 0xdeadbeef;
}

If you run the program now, first of all the valgrind warning goes away, and second the file contents show:

$ od -x file1
0000000 92c0 04c2 0000 0000 ffff ffff beef dead
0000020

The value of the pad variable now shows up in place of the previous zeros.

This all happens because the compiler is trying to make the size of your struct an even multiple of the machine's word size, which in your case appears to be 8 bytes (64 bits).

You probably don't want to include the superfluous pad variable, so your other option to suppress the warning is to clear the entire struct from the start:

typedef struct {
    unsigned char * ustr;
    int height;
}node;

void node_init(node * n, int r) {

    int i;

    /* Clear node struct to suppress valgrind warnings */
    memset(n, 0, sizeof(node));
    n->ustr = malloc((r + 1) * sizeof(unsigned char));
    for (i = 0; i < r; i++) {
        (n->ustr)[i] = random() & 0xff;
    }
    (n->ustr)[r] = 0;
    n->height = -1;
}

This also suppresses the valgrind warning because now you are initializing the padding bytes, and the contents of the extra bytes in the file go back to 0, except now you're explicitly setting them to that rather than relying on default initialization:

$ od -x file1
0000000 92c0 04c2 0000 0000 ffff ffff 0000 0000
0000020

EDIT:

One other experiment to make this clear. Add the following to your main() and run it again:

printf("sizeof unsigned char *: %d\n", sizeof(unsigned char *));
printf("sizeof int: %d\n", sizeof(int));
printf("sizeof node: %d\n", sizeof(node));

On a 64-bit Intel Linux I see:

$ ./writer 
sizeof unsigned char *: 8
sizeof int: 4
sizeof node: 16

The struct is larger than the sum of its parts and you were writing the whole thing, but only initializing part.

EDIT 2:

In response to comment below about fixing the issue that the pointer is being written rather than what it points to, you can address this by writing the fields of the struct individually rather than writing the whole struct itself. Incidentally, this will also fix the original valgrind problem in a different way, because you will no longer be writing the padding bytes. So your main would end up looking like this:

int main() {
    FILE* file_ptr = fopen("file1", "w+");
    node n0;
    node_init(&n0,2);
    fwrite(n0.ustr, strlen(n0.ustr), 1, file_ptr);
    fwrite(&n0.height, sizeof(n0.height), 1, file_ptr);
    fclose(file_ptr);
    node_destroy(&n0);

    return 0;
}

And if you run it and look at the file, it no longer contains your 8 byte pointer, but only the two bytes of data it points to:

$ od -x file1 
0000000 c667 ffff ffff
0000006