CODEWITHSUNDEEP
clinuxmemory-managementinitializationvalgrind
Niklas R
Niklas R

Reputation: 16860

Valgrind, "uninitialized value(s)" error

In my C program, I'm allocating memory using malloc() which does, in contrast to calloc(), not initialize the memory and it might still contain garbage. Mostly, in context of the allocation, I do not make any changes to the memory allocated by malloc(). (For example in a function to initialize a struct that contains a buffer, I do not make changes to the buffer's memory, but later on).

Valgrind gives me a lot of theese errors:

I am sure to never read from memory that was not initialized in these cases.

Should I ignore them or is it better to initialize the memory on allocation? In case I should ignore them, how can I deactivate this error message in Valgrind?

Example 1:

==4253== Conditional jump or move depends on uninitialised value(s)
==4253==    at 0x408EB8E: vfprintf (vfprintf.c:1624)
==4253==    by 0x4093C2E: printf (printf.c:35)
==4253==    by 0x40624D2: (below main) (libc-start.c:226)
==4253==  Uninitialised value was created by a heap allocation
==4253==    at 0x402BE68: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==4253==    by 0x8048938: gk_StreamBufferNode_init (stream.c:101)
==4253==    by 0x8048D0D: gk_Stream_bufferWriteProc (stream.c:252)
==4253==    by 0x8048665: main (main.c:21)

Code:

int gk_StreamBufferNode_init(gk_StreamBufferNode* node, int buffer_size,
                             gk_AllocProc malloc) {
    node->buffer = malloc(buffer_size);     // line 101
    if (node->buffer == NULL) {
        return GKIT_FAILEDALLOC;
    }
    node->next = NULL;
    return GKIT_NOERR;
}

Example 2:

==4253== Conditional jump or move depends on uninitialised value(s)
==4253==    at 0x402DA39: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==4253==    by 0x8048C6E: gk_Stream_bufferWriteProc (stream.c:230)
==4253==    by 0x8048665: main (main.c:21)
==4253==  Uninitialised value was created by a heap allocation
==4253==    at 0x402BE68: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==4253==    by 0x8048CE0: gk_Stream_bufferWriteProc (stream.c:248)
==4253==    by 0x8048665: main (main.c:21)

Code:

    /* ... */
    int available_bytes = binfo->buffer_size - bnode->filled;
    int bytes_to_go = size * count;
    int offset = 0;
    int node_offset = 0;
    gk_StreamBufferNode* new_node;
    void* destination = NULL;
    void* source = NULL;

    while (bytes_to_go > 0) {
        destination = bnode->buffer + bnode->filled + node_offset;
        source = buffer + offset;
        if (available_bytes > bytes_to_go) {
            memcpy(destination, source, bytes_to_go);    // line 230
            bnode->filled += bytes_to_go;
            offset += bytes_to_go;
            node_offset = bytes_to_go;
            bytes_to_go = 0;
        }
        else {
            memcpy(destination, source, available_bytes);
            offset += available_bytes;
            node_offset = 0;
            bytes_to_go -= available_bytes;
            bnode->filled += available_bytes;

            #ifdef DEBUG
                assert(bnode->filled == bnode->buffer_size);
            #endif // DEBUG

            // Allocate a new buffer node.
            new_node = (gk_StreamBufferNode*) malloc(sizeof(gk_StreamBufferNode));    // line 248
            if (new_node == NULL) {
                return GKIT_FAILEDALLOC;
            }
            int success = gk_StreamBufferNode_init(new_node, binfo->buffer_size,
                                                   malloc);
            if (success <= GKIT_ERROR) {
                free(new_node);
                return GKIT_FAILEDALLOC;
            }
            bnode->next = new_node;
            bnode = new_node;
            available_bytes = binfo->buffer_size;
        }
    }

Upvotes: 5

Views: 32869

Answers (2)

Jens Gustedt
Jens Gustedt

Reputation: 78903

In both cases you just allocate memory without initializing it. The easiest way is to use calloc instead of malloc to zero it out. This may be a good strategy for simple cases, e.g if you later use a buffer as a string that is to be printed. For more complicated use cases assign values to the individual fields, or even better if you have C99 assign the whole structure from a compound literal:

toto * t = malloc(sizeof(*t));    
*t = (toto){ 0 };

Upvotes: 6

Hasturkun
Hasturkun

Reputation: 36402

Your code should not be expecting uninitialized memory to contain any value, so having conditional jumps dependent on these shows serious problems.

You should either be initializing the memory (to some known value, eg. 0), or not refer to its contents unless they have been initialized.

Upvotes: 3

Related Questions