Is there a bug here in the code that uses fread

Question

int main()
{
    FILE *read_fp;
    char buffer[BUFSIZ + 1];
    int chars_read;

    memset(buffer, '\0', sizeof(buffer));
    read_fp = popen("cat popen*.c | wc -l", "r");
    if (read_fp != NULL) {
        chars_read = fread(buffer, sizeof(char), BUFSIZ, read_fp);
        while (chars_read > 0) {
            buffer[chars_read - 1] = '\0';
            //buffer[chars_read] = '\0';
            printf("Reading:-\n %s\n", buffer);
            chars_read = fread(buffer, sizeof(char), BUFSIZ, read_fp);
        }
        pclose(read_fp);
        exit(EXIT_SUCCESS);
    }
    exit(EXIT_FAILURE);
}

fread() returns the number of items successfully read

I think the following line should be changed from

buffer[chars_read - 1] = '\0';

to

buffer[chars_read] = '\0';

Do I get it right?

Answer 1

Are you tring to NULL terminate the string read? If so, then yes, buffer[chars_read] = '\0'; would be the way to do that.

Other than that, the structure of your loop could be easier understood if you did it as

do (
    chars_read = fread(....);
    if ( chars_read > 0) { 
       buffer[chars_read] = '\0';
       printf(....);
    }
} while (chars_read > 0);

or more traditional

while ((chars_read = fread(....)) > 0) {
       buffer[chars_read] = '\0';
       printf(....);
}

Answer 2

I believe the code is intended to remove the final newline from the wc -l output, which is why it's writing to where the newline is at the end of the string. It's still not especially sane code, because you'd have to have a very small BUFSIZ or the wrong wc in your $PATH to loop, and if it does loop then overwriting the newline should only happen on the final pass (equivalently and testably, it should check that buffer[chars_read - 1] == '\n').