CODEWITHSUNDEEP
cmemoryvalgrind
MortalMan
MortalMan

Reputation: 2612

Valgrind complaining about call to fgets

I have this code:

int main(int argc, char const *argv[])
{
    FILE *fp = NULL;
    char *buffer = malloc(sizeof(char) * 150);
    char roomElements[150];
    char *roomSize = NULL;
    int i = 1;
    int *size = NULL; // does this need to be malloced?
    char input = '\0';

    check(argc == 2, "Please enter two arguments");

    fp = openFile((char*)argv[1], "r");

    initscr(); // ncurses
    noecho();


    /*Draws rooms and items*/
    while (fgets(buffer, 150, fp)) { // line 25
        removeNewLine(buffer);
        strcpy(roomElements, strchr(buffer, ' '));
        roomSize = strtok(buffer, " "); // get the room size
        size     = getRoomSize(roomSize, i); // convert room size to int pointer
        drawRoom(size, i); // draw the room
        tokenizeRoom(roomElements, i); // draw the elements
        i++;
        free(size);
    }

    /*This is the user input loop*/
    do {
        input = getch();
        getInput(input);
    } while (input != 'q');

    free(buffer);
    fclose(fp);
    endwin();

    return 0;
error:
    return -1;
}

Valgrind output:

==74014== Memcheck, a memory error detector
==74014== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==74014== Using Valgrind-3.11.0.SVN and LibVEX; rerun with -h for copyright info
==74014== Command: ./bin/rogue assets/room.txt
==74014== 
==74014== Invalid read of size 32
==74014==    at 0x10043EC1D: _platform_memchr$VARIANT$Haswell (in /usr/lib/system/libsystem_platform.dylib)
==74014==    by 0x10023080A: fgets (in /usr/lib/system/libsystem_c.dylib) 
==74014==    by 0x100000E10: main (main.c:25)
==74014==  Address 0x100918a40 is 32 bytes before a block of size 4,096 in arena "client"                                                       
==74014== Conditional jump or move depends on uninitialised value(s)
==74014==    at 0x10043EC7A: _platform_memchr$VARIANT$Haswell (in /usr/lib/system/libsystem_platform.dylib)
==74014==    by 0x10023080A: fgets (in /usr/lib/system/libsystem_c.dylib)
==74014==    by 0x100000E10: main (main.c:25)
==74014==  Uninitialised value was created by a heap allocation
==74014==    at 0x100008601: malloc (vg_replace_malloc.c:303)
==74014==    by 0x100233836: __smakebuf (in /usr/lib/system/libsystem_c.dylib)
==74014==    by 0x100236E99: __srefill0 (in /usr/lib/system/libsystem_c.dylib)
==74014==    by 0x100236F94: __srefill (in /usr/lib/system/libsystem_c.dylib) 
==74014==    by 0x1002307DA: fgets (in /usr/lib/system/libsystem_c.dylib)

What could I be doing wrong? Everything is initialized, I am at a loss. No other answers on this topic help me. I am not sure why it complains about the fgets call either.

2nd edit, I will show some more of my code. Here is openFile()

FILE *openFile(char *file, char *mode) {
    FILE *fp = NULL;

    fp = fopen(file, mode);

    check(fp, "Could not open %s", file);

    return fp;
}

Here is getRoomSize

int *getRoomSize(char *size, int roomNum) { // rows x columns
    int row, col;
    char strRow[5], strCol[5];
    int *roomSize = malloc(sizeof(int) * 2);

    check(roomNum < 7 && roomNum > 0, "Invalid room to be drawn");

    /*Convert string to int*/
    strcpy(strRow, strtok(size, "X"));
    strcpy(strCol, strtok(NULL, "X"));
    row = atoi(strRow);
    col = atoi(strCol);

    roomSize[0] = row;
    roomSize[1] = col;

    return roomSize;
}

EDIT 3: Here is my MCVE. It reproduces the conditional jump and uninitialized value errors:

int main(int argc, char const *argv[])
{
    char *buffer= malloc(sizeof(char) * 150);
    char roomElements[300] = {0};
    int i = 1;
    FILE *fp = fopen(argv[1], "r");

    if (!fp) {
        printf("no\n");
        exit(0);
    }

    while (fgets(buffer, 150, fp)) {
        strcpy(roomElements, strchr(buffer, ' '));
        i++;
    }

    free(buffer);
    return 0;
}

The file is a .txt file containing this line 6 times: 15X20 dn2 de10 dw11 g4,2 m10,17 M12,7 M4,9 p11,14 w10,10

Upvotes: 1

Views: 1084

Answers (1)

nneonneo
nneonneo

Reputation: 179392

I think this might be a false alarm.

fgets uses memchr internally to check if it's seen a newline yet. Your platform memchr is a highly optimized assembly routine (the name indicates it is specifically optimized for Haswell architectures) which reads 32 bytes at a time to locate the target character (probably using some SSE instructions to do so). At the end of your buffer, memchr runs off the end of your 150-byte allocation and valgrind complains.

This is totally safe; memchr can't possibly touch bad memory if it sticks to aligned 32-byte boundaries since the page allocation size is (at least) 4096 bytes. If it finds the target character in uninitialized memory, it will presumably detect that the located character is past the end of the string it was supposed to search, and return the correct result in that case.

I'd ignore this error. Valgrind does occasionally produce false positives; consider obtaining or making a platform-specific suppressions file to automatically ignore these kinds of false alarms.

Upvotes: 1

Related Questions