Swift
Swift

Reputation: 13188

Conditional jump or move depends on uninitialised values?

I'm getting the following error when I valgrind my C program:

Conditional jump or move depends on uninitialised value(s)  
==7092==    at 0x40298E: main (search.c:214)  
==7092==  Uninitialised value was created by a heap allocation  
==7092==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)   
==7092==    by 0x40211B: createEntry (words.c:113)   
==7092==    by 0x4027D9: parseIndex (search.c:170)   
==7092==    by 0x402903: main (search.c:208)

Normally this would indicate that I wasn't mallocing the right size for the struct, but I dont think that's the case. Here's all the relevant code, let me know what you think!

/* Entry_
 *
 * @param   filename    filename and path
 * @param   frequency   how often the word appears
 * @param   next        next entry in list
 */

struct Entry_ {
    char *filename;
    int frequency;
    struct Entry_* next;
};

typedef struct Entry_* Entry;

Here's the code for the Create Entry Function:

/* createEntry
 *
 * Creates a brand new entry object.
 *
 * @param   filename        the filename where the entry occured
 *
 * @return  success         new Entry
 * @return  failure         NULL
 */

Entry createEntry(char *filename)
{
    Entry ent;

    ent = (Entry) malloc( sizeof(struct Entry_) ); /* This is line 113 in words.c */
    if(ent == NULL)
    {
        fprintf(stderr, "Error: Could not allocate memory for Entry.\n");
        return NULL;
    }

    ent->filename = (char*) malloc( sizeof(char) * (strlen(filename) + 1));
    if(ent->filename == NULL)
    {
        free(ent);
        fprintf(stderr, "Error: Could not allocate memory for Entry.\n");
        return NULL;
    }

    strcpy(ent->filename, filename);

    ent->frequency = 1;

    return ent;
}

Edit: Added code within Search.c

/* parseIndex
 *
 * Function that takes in an inverted index and returns
 * a HashTable containing all the words and their entries.
 * Returns NULL on failure.
 *
 * @param   filename        name of the inverted index
 *
 * @return  success         new HashTable
 * @return  failure         NULL
 */

HashTable parseIndex(char* filename)
{
    HashTable table;
    TokenizerT tok;
    char* str;
    int res;
    Entry ent;

    if(filename == NULL)
    {
        fprintf(stderr, "Error: Cannot parse NULL file.\n");
        return NULL;
    }

    table = createHT(hash, compStrings, destroyString, destroyWord, printWordHT);
    if(table == NULL)
    {
        fprintf(stderr, "Error: Could not allocate space for HashTable.\n");
        return NULL;
    }

    tok = TKCreate(FILE_CHARS, filename);
    if(tok == NULL)
    {
        fprintf(stderr, "Error: Could not allocate space for Tokenizer.\n");
        return NULL;
    }

    str = TKGetNextToken(tok);
    res = strcmp(str, "files");
    free(str);

    if(res != 0)
    {
        fprintf(stderr, "Error: Malformed index file.\n");
        return NULL;
    }

    /* Parse the file list */
    while((str = TKGetNextToken(tok)) != 0 && strcmp(str, "/files") != 0)
    {
        free(str);
        str = TKGetNextToken(tok);

        if(str == 0 || strcmp(str, "/files") == 0)
        {
            fprintf(stderr, "Error: Malformed index file.\n");
            return NULL;
        }

        ent = createEntry(str); /* Line 170 */

        if(file_list == NULL)
        {
            file_list = ent;
        }
        else
        {
            ent->next = file_list;
            file_list = ent;
        }

        free(str);
    }

    free(str);

    TKDestroy(tok);
    tok = NULL;

    return table;
}

int main( int argc, char** argv )
{
    HashTable table;
    Entry curr, next;
    int i;

    /* Validate the inputs */
    if( (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'h') || argc != 2 )
    {
        fprintf(stderr, "Usage: %s <inverted-index filename>\n", argv[0]);
        return 1;
    }

    file_list = NULL;

    table = parseIndex(argv[1]); /* Line 208 */
    assert(table != NULL);

    curr = file_list;
    i = 0;

    while(curr != NULL)
    {
        next = curr->next;

        printf("[%i]: %s\n", i, curr->filename);

        free(curr->filename);
        free(curr);

        curr = next;
    }

    destroyHT(table);
    table = NULL;

    return 1;
}

Upvotes: 2

Views: 3536

Answers (1)

user25148
user25148

Reputation:

Your createEntry function does not initialize the next field. The actual point where something is being used unitialized is outside the code you've shown (on line 214 of search.c), but from what you have shown, my guess would be the next field.

Some other nitpicks:

  • It's often confusing to hide the pointer inside a typedef. It makes it unclear to the reader whether you've forgotten to use a pointer or not. Sometimes it's unclear to the writer, too, which would result in a bug. Thus I recommend you change the Entry typedef to be like this: typedef struct Entry_ Entry;
  • The underscore at the end of the name of the struct is unnecessary, and looks like a typo. Thus, it would be better style to call it just struct Entry.
  • In C, there is no need to typecast the result of malloc, and doing so can hide a missing #include <stdlib.h>
  • sizeof(char) is by definition 1 in C, so I would leave that out in the malloc call.

Other than those things, the function looks good to me. Kudos for checking and reporting errors.

Upvotes: 6

Related Questions