C: EOF detection & fgetc

1
votes

OBJECTIVE: How to implement EOF successfully to stop the infinite loop?

part where the function is called:

do {
    pId = readInputField(fptr, DELIMITER_SPACE);
    pName = readInputField(fptr, DELIMITER_SEMICOLON);
    pDob = readInputField(fptr, DELIMITER_SPACE);
    pHobbyList = readInputField(fptr, DELIMITER_NEWLINE);
} while (NULL != pId
         && NULL != pName
         && NULL != pDob
         && NULL != pHobbyList);

function definition:

char* readInputField(FILE* fPtr, const char delimiter) {
    int numCharRead;
    char bufferString[MAX_LENGTH_INPUT];
    char *pBufferString;

    numCharRead = 0;

    // flush: if spaces are found
    ' ' == (bufferString[numCharRead] = fgetc(fPtr)) ? 0 : numCharRead++;

    // get chracter array before delimiter
    while (delimiter != bufferString[numCharRead - 1]
            && numCharRead < MAX_LENGTH_INPUT) {
        bufferString[numCharRead++] = fgetc(fPtr);
    }

    // exclude delimiter from the string
    bufferString[numCharRead - 1] = '\0';

    printf("numCharRead=  \"%d\"\n", numCharRead);

    printf("delimiter:    \"%c\"\n", delimiter);
    printf("bufferString: \"%s\"\n", bufferString);

    pBufferString = malloc(sizeof(char*) * strlen(bufferString));

    /* deleted:
    pBufferString = bufferString;
    return EOF == bufferString[numCharRead - 1] ? NULL : pBufferString;
    */
}

sample input:

VIC Lee, Victoria; 02/25/90 Knitting;Photography;Dance;

sample output:

numCharRead=  "4"
delimiter:    " "
bufferString: "VIC"
numCharRead=  "14"
delimiter:    ";"
bufferString: "Lee, Victoria"
numCharRead=  "9"
delimiter:    " "
bufferString: "02/25/90"
numCharRead=  "28"
delimiter:    "
"
bufferString: "Knitting;Photography;Dance;"

// after this, infinite loop begins with garbage data

My call is to look at return statement above. For some reason, it does not detect if it is EOF.

Any help is appreciated! Thanks!

updated: Thanks @JoachimPileborg! I have updated my code below:

  // check for EOF
    if(bufferString[numCharRead-1] == EOF) {
        return NULL;
    } else {
        pBufferString = malloc(sizeof(char*));
        strcpy(pBufferString, bufferString);
        return pBufferString;
    }
2
ceoffgetc
Remember that fgetc returns an int not a char. - Some programmer dude
Oh, and you have an ever worse problem: You return a pointer to a local variable. - Some programmer dude
@JoachimPileborg Thanks. I updated the code above to fix pointer problem. Regarding fgetc, I don't even get a warning, and the results are just as expected. What would you suggest to replace fgetc? - J. Berman
You allocate almost four or eight times more memory than needed. sizeof(char*) is 4 or 8. It's enough to allocate strlen(...) + 1 bytes. And you should use strcpy to copy the string, now you change the pointer to point to the local variable (so still return a pointer to a local variable). - Some programmer dude
' ' == (bufferString[numCharRead] = fgetc(fPtr)) ? 0 : numCharRead++;... this is a really convoluted way of expressing if ((bufferString[numCharRead] = fgetc(fPtr))) numCharRead++;. - dreamlax

2 Answers

2
votes

Check for EOF where you read from file, as

// flush: if spaces are found
' ' == (bufferString[numCharRead] = fgetc(fPtr)) ? 0 : numCharRead++;
if(bufferString[numCharRead-1] == EOF)
    return NULL;

while (delimiter != bufferString[numCharRead - 1]
            && numCharRead < MAX_LENGTH_INPUT) {
    bufferString[numCharRead++] = fgetc(fPtr);
    /* check for EOF */
    if(bufferString[numCharRead-1] == EOF)
        return NULL;
}

You also have problems that mentioned in comments by @Joachim Pileborg

  • fgetc returns int not char
  • You are returning bufferString which is local to function.
1
votes

Can't write code in a comment so I post this as an answer.

You have (or at least had) this code in the question:

pBufferString = malloc(sizeof(char*) * strlen(bufferString));
pBufferString = bufferString;
return EOF == bufferString[numCharRead - 1] ? NULL : pBufferString;

In the first line above you allocate memory and makes pBufferString point to that memory.

In the second line you then make pBufferString point to the local array bufferString, so you no longer have a pointer to the memory you allocated with malloc, causing a memory leak (and a probable crash when you later try to free that pointer).

You then return pBufferString, which is now pointing to the local array, leading to undefined behavior, as the stack-memory occupied by the local array is no longer valid after the function returns.

Besides the above problem, you allocate almost four or eight times as much memory as needed. The size of a pointer is four or eight bytes (depending on if you are on a 32 or 64 bit platform), but a char is only one byte. It's enough to use strlen(bufferString) + 1 as the size to allocate:

pBufferString = malloc(strlen(bufferString) + 1);

You need the + 1 because the string terminator character is not included in the string length.