Removing comments from a C source file

Question

I wrote this program to remove comments from a C source file, accepting a file name from the command prompt. Let me know if I missed anything or if there are any improvements that can be made in the program.

#include<stdio.h>
#include<stdbool.h>

void m_cmnt( FILE *fp )
{
    int prev=0 , ch ;

    while ((ch=getc(fp))!=EOF)
    {
        if ( prev== '*' && ch == '/')
            return ;
        else
        prev=ch ;
    }
}

void s_cmnt( FILE *fp )
{
    int ch ;
    while ( (ch=getc(fp))!='\n' && ch!=EOF )
        ;
}

int main ( int c , char **arr )
{
        FILE *fp , *np ;
        int ch , prev=0 ;
        bool String=0 ;

        fp = fopen(arr[1] , "r") ;
        np = fopen("temp.txt" , "w") ;

        if (fp==NULL )
        {
            printf ("Invalid/No Filename given as Argument ! \n") ;
            return 1 ;
        }
        while ((ch=getc(fp))!=EOF )
        {
            /* file pointer currently not inside a string */
            if ( !String )
            {
                if ( ch=='/' )
                {
                    prev=ch ;
                    ch= getc(fp) ;
                    switch(ch)
                    {
                        case '*'  : m_cmnt( fp) ;
                                     break ;
                        case '/'  : s_cmnt( fp) ;
                                     break ;
                        default   :  putc(prev , np) ;
                                     putc(ch , np) ;
                                     break ;
                    }
                }
                else putc( ch ,np ) ;
            }
            else  putc(ch , np) ;

            if ( ch=='\"')
                String = !String ;
            prev = ch ;
        }

    fclose(fp) ;
    fclose(np) ;
    remove(arr[1]) ;
    rename( "temp.txt" , arr[1] ) ;
    return 0 ;
}

Answer 1

Bug: String escaping

In C, "foo \" bar" is a legal string, but it will confuse the state of your program (so " bar" will be seen as not in a string, while everything after the end of the string will be seen as inside a string). You need to handle at least the escape sequence \" within strings.

Bug: Comments as token separators

If you have the (admittedly weird) C:

int/**/bar;

That will parse as int, bar, ; (declaration of an integer called bar). With comments removed as by this program, it will come out as:

intbar;

Which is probably a parse error.

C syntax should always allow arbitrary whitespace around comments, so this could be fixed by always emitting some whitespace after removing a comment. That could make the output rather ugly, so if you wanted to be 'clever' you could try to only insert extra whitespace when necessary.

Bug: potentially out-of-bounds access through arr[1]

c should be checked to make sure it's greater than 1 before accessing through arr[1].

Suggestion: State machine

The above-mentioned bug is part of a larger issue: there's a lot of "hidden" state to do with things like "what was the previous character" etc.

The nice thing about most programming languages (including C) is that they are designed so that lexical analysis (which is what you need to do to remove comments) is very simple, and can be done with a finite-state automaton (often just called a state machine).

In some sense your program is already a finite-state machine, with the state being the combination of the String flag, the prev variable, and which function you're in---but it's a little hard to keep track of. Introducing an explicit state machine will make the code clearer, and make it easier to handle cases like escaped quotes in strings.

There are various implementation patterns for state machines; a common one for small/simple state machines, such as this one, is to use an enum to name some states:

enum { START, COMMENT_START, COMMENT, COMMENT_END, LINE_COMMENT, STRING, STRING_ESCAPE, };

and then use a switch statement to behave differently depending on the state:

int state = START;

...

switch (state) {
case START:
    if (c == '/') state = COMMENT_START;
    /* other branches */
    break;
case COMMENT_START:
    if (c == '*') state = COMMENT;
    else if (c == '/') state = LINE_COMMENT;
    /* other branches */
    break;
    .
    .  /* etc */
    .
}

Convention: standard names for arguments to main

The almost universal convention is for the signature of main to be

int main(int argc, char *argv[])

There isn't anything inherently wrong with using different names for those variables, but using argc/argv makes it slightly easier for other people to read.

Pedantry: whitespace

The use of whitespace around punctuation seems a little...haphazard. (For example, some parentheses have spaces around them, while others have none; some commas have spaces both before and after, some have one, some have none). Regardless of which style you prefer (which is an opinion thing), the inconsistency makes it more difficult to read.

Feature: not-in-place modification

A useful behaviour (and the behaviour I'd expect) would be to be able to call

commentremover foo.c

and have it print the comment-removed version of the program to STDOUT, without clobbering foo.c.

Answer 2

You don't handle pre-processor related things such as \.

Consider the case where

int x = 0;
//setting \
x to 0

The backslash at the end of a line should "escape" the newline character that follows. But because you skip all characters until \n, you ignore the escaping and would return the file as

int x = 0;
x to 0

Which would no longer compile.

Of course, if we were really evil, we would do something like this:

File 1

#define WEIRD_COMMENT //comment \\

File 2

#include <file1.h>

WEIRD_COMMENT
//this is a weird comment
int main(void)
{
 ...

And your code would again break things, by removing the //this is a weird comment line, causing the WEIRD_COMMENT inclusion to turn int main into a comment... At this point I worry about the feasibility of your solution - there's no way for you to strip this sort of comments properly, I think.

Then there's also trigraphs, which are sequences like ??/, which converts to \. Take a look on wikipedia for a full view of the madness that is trigraphs and digraphs.