Unexpected Behaviour in C, string manipulation

Question

Here is my full code:

// Write a program that reads twelve strings from fruits.txt,
// reverses each string and outputs them to output/outfile.txt.

#include <stdio.h>
#include <string.h>

void main() {

    FILE *fruitFile;
    FILE *outputFile;
    char fruit[32];
    char revFruit[32];

    fruitFile = fopen( "fruits.txt", "r" );
    outputFile = fopen( "output.txt", "w" );

    if (fruitFile == NULL) {
        printf("there was an error opening fruits.txt\n");
    } else if (outputFile == NULL) {
        printf("there was an error opening output.txt\n");
    } else {
        
        while(!feof(fruitFile)) {
            
            int j = 0;
            size_t i = 0;

            for (i = 0; i < strlen(revFruit); i++) {
                revFruit[i] = '\0';
                fruit[i] = '\0';
            }

            fscanf(fruitFile, "%s\n", &fruit);

            for (i = strlen(fruit) - 1; i >= 0; i--) {
                revFruit[j] = fruit[i];
                if(i == 0) break;
                j++;
            }

            printf("%s\n", revFruit);

            fprintf(outputFile, "%s\n", revFruit);
        }

        fclose(fruitFile);
        fclose(outputFile);
    }
}

It is supposed to read from fruits.txt and write the fruits in reverse into output.txt, everything works fine but for some reason once a word is bigger than the others it messes up the whole revFruit string:

Test Input:

apple
mango
banana
grapes
pomegranate
blueberry
guava
apricot
cherry
orange
peach
pear

Test Output:

elppa
ognam
ananab
separg
etanargemop��
yrrebeulbop��
avaugeulbop��
tocirpalbop��
yrrehcalbop��
egnaroalbop��
hcaepoalbop��
raeppoalbop��

Which doesn't make sense to me. I am clearing both strings and I even tried printing revFruit it is empty right before the for loop, but once inside it has leftovers from the last fruit. I am new to C and kind of a noob so any explanation appreciated!

perhaps , but see [Why is “while( !feof(file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feoffile-always-wrong) — yano, Aug 26 '22 at 18:30
Are you compiling with warnings? `gcc` reports at least two pretty concerning ones. — Stephen Newell, Aug 26 '22 at 18:32
`revFruit` is uninitialized, there's no telling what `strlen(revFruit)` will return on first call. It looks like you want `sizeof` there instead, but would be easier to delete the loop entirely and initialize with `char fruit[32] = {0};`, etc. — yano, Aug 26 '22 at 18:32
Remove `if(i == 0) break;` and add `revFruit[j] = 0;` after the loop to properly terminate the string. The loop to set the arrays to all 0s is flawed, instead of `strlen(revFruit)` it should be `sizeof(revFruit)`. A better way would be `memset(fruit, 0, sizeof(fruit)); memset(revFruit, 0, sizeof(revFruit));` so there wouldn't be a problem if the arrays were different sizes. — Retired Ninja, Aug 26 '22 at 18:33
Style suggestion: (and it's opinion) have your checks on opening the files print an error message (to `stderr`) and then return an exit code other than `0`. That will let you unindent the code in your `else` branch, making things easier to follow. — Chris, Aug 26 '22 at 18:54
Aside: the `void main()` is obsolete and only allowed for backward compatibility. Use `int main(void)`. — Weather Vane, Aug 26 '22 at 19:10
See [What is the effect of trailing white space in a `scanf()` format string?](https://stackoverflow.com/q/19499060/15168) for a discussion of why `"%s\n"` or similar format strings are a really bad idea. In your defence, if the input comes from a file, as it does in your code, the problem is not as dramatic, but if a user might be typing the input, the trailing white space is totally incorrect. Consider using `fgets()` to read lines and then processing those. Consider using `"%31s"` with the space limit to avoid overflows. Always check the return value from `fscanf()`. Do not use `feof()`! — Jonathan Leffler, Aug 26 '22 at 19:24
I didn't find that bug when I compiled and executed the code, but I got a warning: "warning: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘char (*)[32]’ [-Wformat=]". And I believe the code has several issues. — Pedro Amaral Couto, Aug 26 '22 at 19:37
Another warning: "warning: comparison of unsigned expression in ‘>= 0’ is always true [-Wtype-limits]". If you're using gcc, execute it with `-W`. — Pedro Amaral Couto, Aug 26 '22 at 19:45
@WeatherVane It's not even backward compatibility? `void main()` has never been valid. The 1989 ANSI C standard simultaneously introduced the `void` keyword and stated that `main` returns `int`. (Implementation-defined definitions are allowed, so a compiler might legally accept `void main()`, but there's no good reason to use it.) — Keith Thompson, Aug 26 '22 at 20:31

score 0 · Answer 1 · answered Sep 05 '22 at 14:41

A couple cleanups of the code. Some of the key points in this task:

Don't use the invalid void main().
Use = {0}; to initialize arrays.
Use break; to end the infinity while loop upon EOF.
Check for the \n at the end of the line read from the file, chomp it with a null.
Add the null to the end of the string at position j, once you copied the word.

New Code:

#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <stdlib.h>

int main() {

    FILE *fruitFile;
    FILE *outputFile;
    char fruit[32] = {0};
    char revFruit[32] = {0};

    fruitFile = fopen( "fruits.txt", "r" );
    outputFile = fopen( "output.txt", "w" );

    if (fruitFile == NULL) {
        printf("there was an error opening fruits.txt\n");
    } else if (outputFile == NULL) {
        printf("there was an error opening output.txt\n");
    } else {

        while (1) {
            char *ret = fgets(fruit, sizeof(fruit), fruitFile);
            if (ret == NULL) {
                if (errno) {
                    printf("there was an error reading fruits.txt: %s\n", strerror(errno));
                    exit(EXIT_FAILURE);
                }
                // end of file
                break;
            }

            int j = 0;
            int i = strlen(fruit);
            if (fruit[i-1] == '\n') {
                fruit[i-1] = 0;
            }

            for (int i = strlen(fruit) - 1; i >= 0; i--) {
                revFruit[j] = fruit[i];
                j++;
                if (i == 0) {
                    break;
                }
            }
            revFruit[j] = 0;
            printf("%s\n", revFruit);
            fprintf(outputFile, "%s\n", revFruit);
        }
        fclose(fruitFile);
        fclose(outputFile);
    }
}

Output:

% ./un
elppa
ognam
ananab
separg
etanargemop
yrrebeulb
avaug
tocirpa
yrrehc
egnaro
hcaep
raep

Unexpected Behaviour in C, string manipulation

1 Answers1