Search code examples
cloopsreversec-stringsfunction-definition

Reverse line function in C works unpredictably


I'm doing the exercises from C Programming Language, the one where you need to make a function to reverse a line. So I did and it works sometimes. But only some times. With the same test it gives different results. I really don't get it, and would appreciate some help. 3 tries out of 4 it would print around 150 spaces and 1 out of 4 it would print the reversed line just like I wanted, though with some junk in the end for some reason. I was thinking of doing it with pointers, but couldn't figure them out as of now. Here's my code:

#include <stdio.h>

void reverse(char theline[150]){
    int i, j;
    char tmp[150];
    for (i = 0; theline[i] != 0; i++){
        tmp[i] = theline[i];
    }
    for (j = 0; i >= 0; j++){
        theline[j] = tmp[i];
        i--;
    }
}

int main() {
    char line[150];
    char c;
    int counter = 0;
    do {
        counter = 0;
        while (((c = getchar()) != '\n') && (c != EOF)) { //one line loop
            line[counter] = c;
            counter++;
        }
        if (counter > 80){
            reverse(line);
            printf("%s\n", line);
        }
    } 
    while (c != EOF);

    return 0;
}

I compile it with "gcc -g -Wall program -o test" and the compiler doesn't give me any errors or warnings. My OS is Ubuntu and I test it with "./test < longtext.txt". This text file has a few lines of different length.


Solution

  • After this loop

        while (((c = getchar()) != '\n') && (c != EOF)) { //one line loop
            line[counter] = c;
            counter++;
        }
    

    the character array line does not contain a string because the stored characters are not appended with the terminating zero character '\0'.

    So this loop within the function

    for (i = 0; theline[i] != 0; i++){
        tmp[i] = theline[i];
    }
    

    invokes undefined behavior.

    You need to append the array with the terminating zero character '\0'.

    But even if the passed character array will containe a string the second for loop

    for (i = 0; theline[i] != 0; i++){
        tmp[i] = theline[i];
    }
    for (j = 0; i >= 0; j++){
        theline[j] = tmp[i];
        i--;
    }
    

    writes the terminating zero character '\0' in the first position if the array theline. As a result you will get an empty string.

    Also the function shall not use the magic number 150 and an auxiliary array.

    Pay attention to that the variable c should be declared as having the type int. In general the type char can behave either as the type signed char or unsigned char depending on compiler options. If it will behave as the type unsigned char then this condition

    c != EOF
    

    will always evaluate to true.

    Without using standard C string functions the function can be declared and defined the following way

    char * reverse( char theline[] )
    {
        size_t i = 0;
    
        while ( theline[i] != '\0' ) i++;
    
        size_t j = 0;
    
        while ( j < i )
        {
            char c = theline[j];
            theline[j++] = theline[--i];
            theline[i] = c;
        }
    
        return theline;
    }
    

    Here is a demonstration program

    #include <stdio.h>
    
    char * reverse( char theline[] )
    {
        size_t i = 0;
    
        while ( theline[i] != '\0' ) i++;
    
        size_t j = 0;
    
        while ( j < i )
        {
            char c = theline[j];
            theline[j++] = theline[--i];
            theline[i] = c;
        }
    
        return theline;
    }
    
    int main( void ) 
    {
        char s[] = "Hello World!";
    
        puts( s );
        puts( reverse( s ) );
    }
    

    The program output is

    Hello World!
    !dlroW olleH