Problems with reversing line into file

Question

This code does reverse and put result in file, but not fully correct. Like, some issues with detecting line breaker or sth. Here's an example:

Source:

This is a line.
This is another line.
Quick brown fox jumps over a lazy dog!

Result:

(blank line)
.enil a si sihT
.enil rehtona si sihT!god yzal a revo spmuj xof nworb kciuQ

#include <stdio.h>
#include <string.h>
char *reverse (char *str)
{
    char *begin, *end, c;
    if (!str || !(*str))
        return NULL;
    for (begin=str, end=str+strlen(str)-1; begin<end; begin++, end--)
    {
        c=*begin;
        *begin=*end;
        *end=c;
    }
    begin=str+strlen(str)+1; *begin='\0'; //??
    return str;
}
void main(void)
{
    char line[1000];
    FILE *fsrc, *frslt;
    fsrc=fopen("source.txt", "r");
    if (fsrc==NULL) return;
    frslt=fopen("result.txt", "w");
    while (!feof(fsrc))
    {
        fgets (line, 1000, fsrc);
        fputs (reverse(line), frslt);
    }
    fclose(fsrc);
    fclose(frslt);
}

[The feof in the while loop is wrong - read this](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) — Ed Heal, Oct 18 '16 at 19:15
The code is working fine, but it's reversing the lines too well. What is happening is that the new-line feed (`\n`) is being reverse as well so the new-line feed is being placed at the beginning of each line, but "The quick brown fox..." has no line feed at the end, so that's why lines 2 and 3 are smooshed together. Trying using `\n` as the end of the line to be reversed. — Eli Sadoff, Oct 18 '16 at 19:15
you don't need it since you're working in-place: `begin=str+strlen(str)+1; *begin='\0';` and you're adding too much (no problem since buffer is bigger but still..) — Jean-François Fabre, Oct 18 '16 at 19:16
Personally, I would [remove the newline that fgets puts in the buffer](https://stackoverflow.com/questions/2693776), and then output a newline after outputting the reversed text. — user3386109, Oct 18 '16 at 19:26

score 3 · Accepted Answer · answered Oct 18 '16 at 19:32

A couple of comments/nitpicks, which may or may not solve your problem :)

if (!str || !(*str))
    return NULL;

Don't do that. Don't return NULL on empty strings, fputs() will barf. In my experience, it's better to a) assert that the str pointer is non-null, b) return the empty string.

begin=str+strlen(str)+1; *begin='\0'; //??

There should be no need to terminate the string, since it's already terminated.

void main(void)

Nah, main() returns an int.

while (!feof(fsrc))

This won't work. You need to do some IO before you can test for feof()/ferror(). IMHO it's better to simply loop on fgets().

while (fgets(line, sizeof line, fsrc) {
     ...
 }

It may be a good idea to drop the input and output files, and simply read from stdin and write to stdout, at least while testing. The functionality you're trying to implement is already available in a UNIX shell (man rev). Using stdin/stdout makes it easier to test and compare results with the results from rev.

Also, keep in mind that fgets() won't remove the \n from the string. input like "foo\n" becomes "\noof", which is probably not what you want.

Here's a snippet which illustrates my comments in code. It doesn't solve all problems, but should be sufficient to get you going.

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

void reverse(char *str)
{
    char *begin, *end, c;
    size_t n;

    assert(str != NULL);

    n = strlen(str);
    if (n == 0)
        return;

    for (begin = str, end = str + n - 1; begin < end; begin++, end--) {
        c = *begin;
        *begin = *end;
        *end = c;
    }
}

int main(void)
{
    char line[1000];

    while (fgets(line, sizeof line, stdin)) {
        reverse(line);
        fputs(line, stdout);
    }
}

HTH

Problems with reversing line into file

1 Answers1