C: character comparison fails

Question

After succesfully running an entabulator, my detabulator won't pick up on a character comparison that should exit a while loop. After trying "0(tab)8(enter)(ctrl+D)" as input the tab is written correctly as spaces, but after rp is incremented to point to the 8, the while loop that should read the 8 won't exit and I get a seg fault. Here's the code:

#include <string.h>
#include <stdio.h>
#define MAXLINE 100
char doc[9001];
main(int argc, char *argv[])
{
    int max = 0;
    char *rp = doc;
    char *wp = rp;
    char *tf = wp;
    char *lp = doc;

    while ((*(rp++) = getchar()) != EOF);
    *--rp = '\0';
    rp = doc;
    j = 0;
    while  ( (*rp != '\0') && (argc == 1)) {
        if (*rp == '\n') {
            lp = rp + 1;
            *wp++ = *rp++;
        }

        while( (*rp != '\t') && (*rp != '\0') && (*rp != '\n') ) { /*this loops after a tab*/
            *wp++ = *rp++;
        }
        if (*rp == '\t') {
            rp++;
            tf = lp + ((((wp - lp) / 8) + 1) * 8);
            while ((tf - wp) != 0) 
                *wp++ = 's';
        }
    }
    if (*rp == '\0')    
        *wp = '\0';
    printf("%s\n", doc);
}

Answer 1

What I feel is, the below loop is going into infinite loop.

while( (*rp != '\t') && (*rp != '\0') && (*rp != '\n') ) { /*this loops after a tab*/
    *wp++ = *rp++;

This is because, you are checking for rp!= '\t' and so on, but here

if (*rp == '\t') 
{
    rp++;
    tf = lp + ((((wp - lp) / 8) + 1) * 8);
    while ((tf - wp) != 0) 
        *wp++ = 's';
}

you are filling the doc array with char 's' and which is over writing '\t' also, so the above loop is going to infinite.

Answer 2

There are some as yet unexplored problems with the initial input loop.

You should never risk overflowing a buffer, even if you allocate 9001 bytes for it. That's how viruses and things break into programs. Also, you have a problem because you are comparing a character with EOF. Unfortunately, getchar() returns an int: it has to because it returns any valid character value as a positive value, and EOF as a negative value (usually -1, but nothing guarantees that value).

So, you might write that loop more safely, and clearly, as:

char *end = doc + sizeof(doc) - 1;
int c;

while (rp < end && (c = getchar()) != EOF)
    *rp++ = c;
*rp = '\0';

With your loop as written, one of two undesirable things happens:

• if char is an unsigned type, then you will never detect EOF.
• if char is a signed type, then you will detect EOF when you read a valid character (often ÿ, y-umlaut, LATIN SMALL LETTER Y WITH DIAERESIS, U+00FF).

Neither is good. The code above avoids both problems without needing to know whether plain char is signed or unsigned.

Conventionally, if you have an empty loop body, you emphasize this by placing the semicolon on a line on its own. Many an infinite loop has been caused by a stray semicolon after a while condition; by placing the semicolon on the next line, you emphasize that it is intentional, not accidental.

while ((*(rp++) = getchar()) != EOF);

while ((*(rp++) = getchar()) != EOF)
    ;