5
\$\begingroup\$

Challenge posted 5 Dec 2020 to comp.lang.c This program is essentially the same as my Version 8 posted to that thread, but with some further cosmetic improvements.

This program reads from stdin (or a named file) and writes its result to stdout, removing C comments from the text. It has to see through any sequences of BACKSLASH NEWLINE when detecting comments, but reproduce all such line continuation sequences that were not part of a comment. Additionally, the challenge was to use no gotos and reasonably short functions.

The code was originally inspired by a Haskell program and it implements a simple Lisp-ish data structure including a simple garbage collector. I don't want to explain much more because I want the code to speak for itself.

strpcom-v5.c

//strpcom-v5.c:
//$ make strpcom-v5 CFLAGS='-std=c99 -Wall -pedantic -Wextra -Wno-switch'

#ifndef DEBUG_LEVEL
#define DEBUG_LEVEL 0
#endif
static const int debug_level = DEBUG_LEVEL;
static const int handle_roots = 1;


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


typedef union uobject *object;
typedef object list;
typedef enum tag { INVALID, INTEGER, LIST } tag;
#define OBJECT(...) new_( (union uobject[]){{ __VA_ARGS__ }} )

union uobject { tag t;
       struct { tag t; int i, continues; } Int;
       struct { tag t; object a, b; } List;
};

typedef struct record {
  int mark;
  struct record *prev;
  object *handle;
  union uobject o;
} record;


static void     init_patterns();
static void     cleanup();
static list     chars_from_file( FILE *f );
static list     logical_lines( list o );
static list       trim_continue( list o, list tail );
static list     restore_continues( list o );
static list     strip_comments( list o );
static list       single_remainder( list tail );
static list       multi_remainder( list tail );
static int        starts_literal( object a );
static list       literal_val( object a, list o );
static list     skip_quote( object q, list o );
static list     nested_comment( list o );
static void     print( list o, FILE *f );

static object   Int( int i );
static list     cons( object a, object b );
static list     one( object a );
static object   car( list o );
static object   cdr( list o );
static int      eq( object a, object b );
static int      eqint( object a, int i );
static int      match( object pat, object it, object *matched, object *tail );

static object   add_global_root( object o, object *op );
static int      collect( object local_roots );
static void       mark( object ob );
static int        sweep( record **ptr );
static record *   alloc();
static object   new_( object a );
static int      error( char *msg );


record *allocation_list;
object global_roots;
list   slnl; // patterns
list   single,
       multi;
list   starsl;

int
main( int argc, char **argv ){
  init_patterns();
  list input = chars_from_file( argc > 1  ? fopen( argv[1], "r" )  : stdin );
  if(  debug_level >= 2  )
    fprintf( stderr, "input:\n"), print( input, stderr );

  list logical = logical_lines( input );
  if(  debug_level >= 2  )
    fprintf( stderr, "logical:\n"), print( logical, stderr );

  list stripped = strip_comments( logical );
  if(  debug_level >= 2  )
    fprintf( stderr, "stripped:\n"), print( stripped, stderr );

  list restored = restore_continues( stripped );
  if(  debug_level >= 2  )
    fprintf( stderr, "restored:\n");

  print( restored, stdout ), fflush( stdout );
  cleanup( restored ), input = logical = stripped = restored = NULL;
}

void
init_patterns(){
  slnl   = add_global_root( cons( Int( '\\' ), one( Int( '\n' ) ) ), &slnl );
  single = add_global_root( cons( Int( '/' ), one( Int( '/' ) ) ), &single );
  multi  = add_global_root( cons( Int( '/' ), one( Int( '*' ) ) ), &multi  );
  starsl = add_global_root( cons( Int( '*' ), one( Int( '/' ) ) ), &starsl ); 
}

void
cleanup( object restored ){
  if(  debug_level  ){
    if(  debug_level >= 2  ) fprintf( stderr, "@%d\n", collect( restored ) );
    if(  handle_roots  ){ global_roots = NULL; }
    fprintf( stderr, "@%d\n", collect( NULL ) );
  }
}

list
chars_from_file( FILE *f ){
  int c = fgetc( f );
  return  c != EOF  ? cons( Int( c ),
                            chars_from_file( f ) )
       :  one( Int( c ) );
}


list
logical_lines( list o ){
  if(  !o  ) return  NULL;
  if(  debug_level >= 2 && car(o)->Int.i != EOF  )
      fprintf( stderr, "[%c%c]", car(o)->Int.i, car(cdr(o))->Int.i );
  object matched, tail;
  return  match( slnl, o, &matched, &tail )  ? trim_continue( o, tail )
       :  cons( car( o ),
                logical_lines( cdr( o ) ) );
}

list
trim_continue( list o, list tail ){
  if(  debug_level >= 2  ) fprintf( stderr, "@" );
  return  car( tail )->Int.continues = car( o )->Int.continues + 1,
          logical_lines( tail );
}

list
restore_continues( list o ){
  return  !o  ? NULL
       :  car( o )->Int.continues-->0  ?
            cons( Int( '\\' ),
                  cons( Int( '\n' ),
                        restore_continues( o ) ) )
       :  cons( car( o ),
                restore_continues( cdr( o ) ) );
}

list
strip_comments( list o ){
  if(  !o  ) return  NULL;
  if(  debug_level >= 2 && car(o)->Int.i != EOF  )
      fprintf( stderr, "<%c%c>", car(o)->Int.i, car(cdr(o))->Int.i );
  object matched, tail;
  object a;
  return  match( single, o, &matched, &tail )  ? single_remainder( tail )
       :  match( multi, o, &matched, &tail )   ? multi_remainder( tail )
       :  starts_literal( a = car( o ) )       ? literal_val( a, cdr( o ) )
       :  cons( a,
                strip_comments( cdr( o ) ) );
}

list
single_remainder( list tail ){
  if(  debug_level >= 2  ) fprintf( stderr, "@/" );
  object c;
  for(  c = car( tail );
        tail && ! (eqint( c, '\n' ) || eqint( c, EOF ));
        c = car( tail = cdr( tail ) )  )
    ;
  return  eqint( c, '\n' )  ? cons( Int( '\n' ),
                                    strip_comments( cdr( tail ) ) )
       :  tail;
}

list
multi_remainder( list tail ){
  if(  debug_level >= 2  ) fprintf( stderr, "@*" );
  return  cons( Int( ' ' ),
                strip_comments( nested_comment( tail ) ) );
}

int
starts_literal( object a ){
  return  eqint( a, '\'' ) || eqint( a, '"' );
}

list
literal_val( object a, list o ){
  return  cons( a,
                skip_quote( a, o ) );
}

list
nested_comment( list o ){
  if(  !o  ) error( "Unterminated comment\n" );
  if(  debug_level >= 2  )
    fprintf( stderr, "(%c%c)", car( o )->Int.i, car( cdr( o ) )->Int.i );
  object matched, tail;
  return  match( starsl, o, &matched, &tail )  ? tail
       :  eqint( car( o ), EOF )               ? error( "Unterminated comment\n" ),NULL
       :  nested_comment( cdr( o ) );
}

list
skip_quote( object q, list o ){
  if(  !o  ) error( "Unterminated literal\n" );
  object a = car( o );
  return  eqint( a, '\\' )  ? cons( a, 
                                    cons( car( cdr( o ) ),
                                          skip_quote( q, cdr( cdr( o ) ) ) ) )
       :  eq( a, q )        ? cons( a,
                                    strip_comments( cdr( o ) ) )
       :  eqint( a, '\n' )
       || eqint( a, EOF )   ? error( "Unterminated literal\n" ),NULL
       :  cons( a,
                skip_quote( q, cdr( o ) ) );
}

void
print( list o, FILE *f ){
  switch(  o  ? o->t  : 0  ){
    case INTEGER: if(  o->Int.i != EOF  ) fputc( o->Int.i, f );
                  break;
    case LIST: print( car( o ), f );
               print( cdr( o ), f ); break;
  }
}


object
Int( int i ){
  return  OBJECT( .Int = { INTEGER, i } );
}

list
cons( object a, object b ){
  return  OBJECT( .List = { LIST, a, b } );
}

list
one( object a ){
  return  cons( a, NULL );
}

object
car( list o ){
  return  o && o->t == LIST  ? o->List.a  : NULL;
}

object
cdr( list o ){
  return  o && o->t == LIST  ? o->List.b  : NULL;
}

int
eq( object a, object b ){
  return  !a && !b         ? 1
       :  !a || !b         ? 0
       :  a->t != b->t     ? 0
       :  a->t == INTEGER  ? a->Int.i == b->Int.i
       :  !memcmp( a, b, sizeof *a );
}

int
eqint( object a, int i ){
  union uobject b = { .Int = { INTEGER, i } };
  return  eq( a, &b );
}

int
match( object pat, object it, object *matched, object *tail ){
  if(  !pat  ) return  *tail = it,  1;
  if(  pat->t != (it  ? it->t  : 0)  ) return  0;
  switch(  pat->t  ){
    case LIST: {
        object sink;
        if(  match( car( pat ), car( it ), & sink, tail )  )
          return  *matched = it,
                  match( cdr( pat ), cdr( it ), & sink, tail );
      } break;
    case INTEGER:
      if(  eq( pat, it )  ) return  *matched = it,  1;
  }
  return  0;
}


object
add_global_root( object o, object *op ){
  if(  handle_roots  ){
    global_roots = cons( o, global_roots );
    record *r = ((void*)( (char*)o - offsetof( record, o ) ) );
    r->handle = op;
  }
  return  o;
}

void
mark( object ob ){
  if(  !ob  ) return;
  record *r = ((void*)( (char*)ob - offsetof( record, o ) ) );
  if(  r->mark  ) return;
  r->mark = 1;
  switch(  ob  ? ob->t  : 0  ){
    case LIST: mark( ob->List.a ); mark( ob->List.b ); break;
  }
}

int
sweep( record **ptr ){
  int count = 0;
  while(  *ptr  ){
    if(  (*ptr)->mark  ){
      (*ptr)->mark = 0;
      ptr = &(*ptr)->prev;
    } else {
      record *z = *ptr;
      if(  z->handle  ) *z->handle = NULL;
      *ptr = (*ptr)->prev;
      free( z );
      ++count;
    }
  }
  return  count;
}

int
collect( object local_roots ){
  mark( local_roots );
  mark( global_roots );
  return  sweep( &allocation_list );
}

record *
alloc(){
  return  calloc( 1, sizeof(record) );
}

object
new_( object a ){
  record *r = alloc();
  object p = NULL;
  if(  r  ){
    r->prev = allocation_list;
    allocation_list = r;
    p = (void*)( ((char*)r) + offsetof( record, o ) );
    *p = *a;
  }
  return  p;
}

int
error( char *msg ){
  fprintf( stderr, "%s", msg );
  exit( EXIT_FAILURE );
  return 0777;
}

The repository includes a testing script, but due to some bash quirks it reports some false negatives so I don't wish to display the results here. Compiling with DEBUG_LEVEL defined to 1 or higher will run a garbage collection at the end. Running under valgrind including the GC reports zero memory leaks.

Questions: Is is easy to discern the top-level structure? Are the individual functions readable/maintainable? Is it overkill to include a GC that doesn't even run under normal compilation? Are there any functions where it's possible to rewrite in a tail-recursive form to potentially reduce the function call depth?

\$\endgroup\$
3
  • \$\begingroup\$ Why not implement with flex and ten lines of code? \$\endgroup\$ Dec 28 '20 at 20:02
  • 3
    \$\begingroup\$ Well, the challenge was to write it in C and to use no gotos and reasonably short functions. I then got a little sidetracked trying to add a tiny garbage collector. \$\endgroup\$ Dec 28 '20 at 20:08
  • 1
    \$\begingroup\$ Just a side note for a real-world production quality solution, comment chars in string literals ("/*") and replacing comments with whitespace (a/**/b to a b) and conserving line numbers by line feeds in that whitespace. \$\endgroup\$
    – Joop Eggen
    Dec 29 '20 at 4:15
4
\$\begingroup\$

Initial comments

  1. static void cleanup(); is a function declaration, but it is not a prototype. It does not give you the protection that a prototype does. For the cleanup() function, it should be static void cleanup(object restored); — for the other functions without a prototype, the argument list should be explicitly written as function(void) to show that no arguments are expected. You should compile with -Wstrict-prototypes so that such function declarations are rejected.

  2. Personally, I always include static in the function definition as well as the function declaration; I prefer the consistency. Since this is a single source file, I'd make all the file scope (global) variables into static variables too. If it were a multi-file program, then there'd be headers to declare the variables and functions (and any other supporting information needed across files — types, macros, enumerations).

  3. I would include a default: clause or an explicit case INVALID: in the switch so that I did not need to specify -Wno-switch.

  4. Your code does not detect */ appearing outside a /* comment. This isn't a major flaw (a compiler will reject the code). It also doesn't detect /* appearing inside a /* comment. Again, this isn't a major flaw.

  5. Your code doesn't preserve line numbers. It can be a good idea to preserve newlines found in comments. It probably wasn't a design goal, so it isn't a major criticism.

  6. It's possible to avoid trailing white space on output lines, but considerably harder. Again, this probably wasn't a design goal, so it isn't a major criticism.

  7. You don't handle trigraphs. The only one that matters for comment removal is ??/ which is equivalent to a backslash. Since C++ has removed support for trigraphs, and GCC requires the -trigraphs option to enable them, it isn't a major issue, but the fact that they aren't handled should perhaps be documented. There is no need to handle digraphs; digraphs do not affect the interpretation of comments.

  8. You don't detect multiple command-line arguments. IMO, it is not a good idea to ignore extra arguments — either process them or reject them.

  9. You don't error check memory allocations. There's only one function that calls calloc() — no calls to malloc() or realloc() — so it would be easy to add an error check that reports 'out of memory' and stops the program on allocation failure. That's preferable to a crash. (In new_(), you detect if the allocation failed, but simply pass back a null pointer without reporting that the allocation failed.)

  10. You don't error check that the file was opened successfully; that's close to criminal — users give wrong file names or unreadable file names and your program should not crash because they do so.

Yes, I do have a program SCC — Strip C Comments. You can find the code on GitHub under https://github.com/jleffler/scc-snapshots. I ran your code on some of its test cases. SCC also handles C++, including more recent versions, which causes conniptions at times. Raw string literals and punctuation in numeric constants (and binary constants) cause major problems — for example, 0b1101'0011 is a valid C++ binary literal, but looks to a naïve C parser like an incomplete character constant. Sticking with just C is much easier. SCC does not handle digraphs (because it isn't necessary to do so) or trigraphs. If I need to map either digraphs or trigraphs, I have separate programs that can add or remove them (called, with stunning originality, digraphs and trigraphs). They're primarily obfuscatory features these days, though trigraphs addressed a real problem in the days when C90 was defined, and digraphs were added in C99 as preferable to trigraphs. (Hmmm…I need to update the scc-snapshots repository. I've created releases 7.00, 7.10, 7.20, 7.30, 7.40 and 7.50 since I last updated it, though most of the revisions (except for removing trailing blanks) are minor. Oops!) There is some fairly gruesome code in SCC; it wouldn't all necessarily stand up to detailed scrutiny either.

Code style

  • Is it easy to discern the top-level structure?

Not particularly easy.

  • You are over-fond of the comma operator.
  • You use typedef union uobject *object; to hide the pointer — see Stack Overflow on Is it a good idea to typedef pointers?.
  • You slurp the entire file into a list that contains one element for each character in the file. Slurping is a good idea (I've debated doing that for SCC), but I'm not convinced that a list of single characters to represent the whole file is a good idea.
  • You use the LISP-like functions cons, car, cdr — that isn't a common idiom in C.
  • Are the individual functions readable/maintainable?

Some of the functions have odd parts. For example, in function void mark(object ob) you have if (!ob) return; and then later switch (ob ? ob->t : 0). You previously determined that ob is not null, so there's no need to retest it. And a switch with one case and no default: is probably better written as an if statement:

if (ob->t == LIST) {
    mark(ob->List.a);
    mark(ob->List.b);
}

The logical_lines() function is not easily understood.

The trim_continue() function is not easily understood. I think it is written unnecessarily opaquely. It is:

list
trim_continue( list o, list tail )
{
    if (debug_level >= 2)
        fprintf( stderr, "@" );
    return car( tail )->Int.continues = car( o )->Int.continues + 1,
           logical_lines( tail );
}

It would probably be clearer if written as:

list
trim_continue(list o, list tail)
{
    if (debug_level >= 2)
        fprintf(stderr, "@");
    car(tail)->Int.continues = car(o)->Int.continues + 1;
    return logical_lines(tail);
}

It's still opaque, but the comma operator doesn't make anything clearer. It has its uses; this isn't a good one.

Many of the other functions are also hard to understand.

  • Is it overkill to include a GC that doesn't even run under normal compilation?

Yes.

  • Are there any functions where it's possible to rewrite in tail-recursive form to potentially reduce the function call depth?

Probably most of them. I think using lists less extensively would reduce the overhead. I'd expect to read lines into buffers, and then manipulate lines, both a single line at a time and consecutive lines at times. I think that would reduce the overhead.

\$\endgroup\$
5
\$\begingroup\$

I think the code does a lot more to reflect its genealogy than it does to represent the actual task. Despite being written in C, it seems to do its best to embody the old line about every sufficiently complex program including a half-broken implementation of half of LISP.

Simply put, even though it's acceptable to a C compiler, it's not really written even close to how I'd normally expect most people to write C to do a job like this.

It's been years since I did write C much, but as it happens back when I did, I wrote some code to tokenize C source code (I'm not sure, but I think this is from around 1995 or so).

Now make no mistake: I'm not going to claim this is even close to ideal (I'd certainly do it differently if I were going to do the same job today), but I think it's at least quite a bit closer to how I'd typically expect C code to be written.

The big difference I see is that it looks like your code tries to read in the entire source file, decompose it into a giant data structure, then produce its output by regurgitating data from that structure. I think most C programmers (certainly at that time, but I think still largely true today) would prefer to write a filter as being as close to an actual filter as possible--that is, reading in a little data, processing the data, and writing out the processed data as it goes, so even if you had a source file many times larger than your available RAM, it could still process it quite easily. Admittedly, that's not nearly as a serious a problem today as it was 30 years ago when I still wrote quite a bit of C, but I think a lot of C programmers would still be somewhat bothered by the idea of it, unless they gained a significant benefit from storing everything in memory first.

/* get_src.c */
#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <stdlib.h>

#define GET_SOURCE
#include "get_src.h"

static size_t current = 0;

char last_token[MAX_TOKEN_SIZE];

PFILE *parse_fopen(char const *name) {

    PFILE *temp = malloc(sizeof(PFILE));

    if ( NULL != temp ) {
        temp->file = fopen(name, "r");
        memset(temp->peeks, 0, sizeof(temp->peeks));
        temp->last_peek = 0;
    }
    return temp;
}

PFILE *parse_fdopen(FILE *file) {

    PFILE *temp = malloc(sizeof(PFILE));

    if ( NULL != temp) {
        temp->file = file;
        memset(temp->peeks, 0, sizeof(temp->peeks));
        temp->last_peek = 0;
    }
    return temp;
}

int parse_fclose(PFILE *stream) {

    int retval = fclose(stream->file);

    free(stream);
    return retval;
}

static void addchar(int ch) {
/* adds the passed character to the end of `last_token' */

    if ( current < sizeof(last_token) -1 )
        last_token[current++] = (char)ch;

    if ( current == sizeof(last_token)-1 )
        last_token[current] = '\0';
}

static void clear(void) {
/* clears the previous token and starts building a new one. */
    current = 0;
}

static int read_char(PFILE *stream) {
    if ( stream->last_peek > 0 )
        return stream->peeks[--stream->last_peek];
    return fgetc(stream->file);
}

void unget_character(int ch, PFILE * stream) {
    if ( stream->last_peek < sizeof(stream->peeks) )
        stream->peeks[stream->last_peek++] = ch;
}

static int check_trigraph(PFILE *stream) {
/* Checks for trigraphs and returns the equivalant character if there
 * is one.  Expects that the leading '?' of the trigraph has already
 * been read before this is called.
 */

    int ch;

    if ( '?' != (ch=read_char(stream))) {
        unget_character(ch, stream);
        return '?';
    }

    ch = read_char(stream);

    switch( ch ) {
        case '(':   return '[';
        case ')':   return ']';
        case '/':   return '\\';
        case '\'':  return '^';
        case '<':   return '{';
        case '>':   return '}';
        case '!':   return '|';
        case '-':   return '~';
        case '=':   return '#';
        default:
            unget_character('?', stream);
            unget_character(ch, stream);
            return '?';
    }
}

#ifdef DIGRAPH
static int check_digraph(PFILE *stream, int first) {
/* Checks for a digraph.  The first character of the digraph is
 * transmitted as the second parameter, as there are several possible
 * first characters of a digraph.
 */

    int ch = read_char(stream);

    switch(first) {
        case '<':
            if ( '%' == ch )
                return '{';
            if ( ':' == ch )
                return '[';
            break;
        case ':':
            if ( '>' == ch )
                return ']';
            break;
        case '%':
            if ( '>' == ch )
                return '}';
            if ( ':' == ch )
                return '#';
            break;
    }

/* If it's not one of the specific combos above, return the characters
 * separately and unchanged by putting the second one back into the
 * stream, and returning the first one as-is.
 */
    unget_character(ch, stream);
    return first;
}
#endif


static int get_char(PFILE *stream) {
/* Gets a single character from the stream with any trigraphs or digraphs converted 
 * to the single character represented. Note that handling digraphs this early in
 * translation isn't really correct (and shouldn't happen in C at all).
 */
    int ch = read_char(stream);

    if ( ch == '?' )
        return check_trigraph(stream);

#ifdef DIGRAPH
    if (( ch == '<' || ch == ':' || ch == '%' ))
        return check_digraph(stream, ch);
#endif

    return ch;
}

int get_character(PFILE *stream) {
/* gets a character from `stream'.  Any amount of any kind of whitespace
 * is returned as a single space. Escaped new-lines are "eaten" here as well.
 */
    int ch;

    if ( !isspace(ch=get_char(stream)) && ch != '\\')
        return ch;

    // handle line-slicing
    if (ch == '\\') {
        ch = get_char(stream);
        if (ch == '\n') 
            ch = get_char(stream);
        else {
            unget_character(ch, stream);
            return ch;
        }
    }

    /* If it's a space, skip over consecutive white-space */
    while (isspace(ch) && ('\n' != ch))
        ch = get_char(stream);

    if ('\n' == ch)
        return ch;

    /* Then put the non-ws character back */
    unget_character(ch, stream);

    /* and return a single space character... */
    return ' ';
}

static int read_char_lit(PFILE *stream) {
/* This is used internally by `get_source' (below) - it expects the
 * opening quote of a character literal to have already been read and
 * returns CHAR_LIT or ERROR if there's a newline before a close
 * quote is found, or if the character literal contains more than two
 * characters after escapes are taken into account.
 */

    int ch;
    int i;


    clear();
    addchar('\'');

    for (i=0; i<2 && ('\'' != ( ch = read_char(stream))); i++) {

        addchar(ch);

        if ( ch == '\n' )
            return ERROR;

        if (ch == '\\' ) {
            ch = get_char(stream);
            addchar(ch);
        }
    }
    addchar('\'');
    addchar('\0');

    if ( i > 2 )
        return ERROR;

    return CHAR_LIT;
}

static int read_str_lit(PFILE *stream) {
/* Used internally by get_source.  Expects the opening quote of a string
 * literal to have already been read.  Returns STR_LIT, or ERROR if a
 * un-escaped newline is found before the close quote.
 */

    int ch;

    clear();
    addchar('"');

    while ( '"' != ( ch = get_char(stream))) {

        if ( '\n' == ch || EOF == ch )
            return ERROR;

        addchar(ch);

        if( ch == '\\' ) {
            ch = read_char(stream);
            addchar(ch);
        }

    }

    addchar('"');
    addchar('\0');

    return STR_LIT;
}

static int read_comment(PFILE *stream) {
/* Skips over a comment in stream.  Assumes the leading '/' has already
 * been read and skips over the body.  If we're reading C++ source, skips
 * C++ single line comments as well as normal C comments.
 */
    int ch;

    clear();

    ch = get_char(stream);

    /* Handle a single line comment.
     */
    if ('/' == ch) {
        addchar('/');
        addchar('/');

        while ( '\n' != ( ch = get_char(stream))) 
            addchar(ch);       

        addchar('\0');
        return COMMENT;
    }

    if ('*' != ch ) {
        unget_character(ch, stream);
        return '/';
    }

    addchar('/');

    do {
        addchar(ch);
        while ('*' !=(ch = get_char(stream)))
            if (EOF == ch)
                return ERROR;
            else
                addchar(ch);
        addchar(ch);
    } while ( '/' != (ch=get_char(stream)));

    addchar('/');
    addchar('\0');

    return COMMENT;
}

int get_source(PFILE *stream) {
/* reads and returns a single "item" from the stream.  An "item" is a
 * comment, a literal or a single character after trigraph and possible
 * digraph substitution has taken place.
 */

    int ch = get_character(stream);

    switch(ch) {
        case '\'':
            return read_char_lit(stream);
        case '"':
            return read_str_lit(stream);
        case '/':
            return read_comment(stream);
        default:
            return ch;
    }
}

#ifdef TEST

int main(int argc, char **argv)  {
    PFILE *f;
    int ch;

    if (argc != 2) {
        fprintf(stderr, "Usage: get_src <filename>\n");
        return EXIT_FAILURE;
    }

    if (NULL==(f= parse_fopen(argv[1]))) {
        fprintf(stderr, "Unable to open: %s\n", argv[1]);
        return EXIT_FAILURE;
    }

    while (EOF!=(ch=get_source(f))) 
        if (ch < 0) 
            printf("\n%s\n", last_token);
        else
            printf("%c", ch);
    parse_fclose(f);
    return 0;       
}

#endif

Now make no mistake--I'm certainly not trying to hold this up as a paragon of virtue. Its use of a global variable is particularly bothersome (at least to me). It also supports a few things (like trigraphs) the challenge specifically excluded. Its support for digraphs is somewhat broken as well (it's translating them earlier in the translation process than it really should). But I think it's still rather closer to the basic style I think most C programmers would find acceptable. In particular, although it has a lot of other "stuff" (that you'd normally ignore--most of it in the form of static functions, so the rest of the world should never see them) the interface to get_source is simple enough that it's pretty trivial to use the tokenizer for a fairly wide variety of tasks (e.g., writing a simple indent program). I'll admit, I'm a bit uncertain about it, but at first glance, it seems like that's rather less true of your code.

\$\endgroup\$
2
  • \$\begingroup\$ Is there a portability issue with <stdio.h>:ungetc()? Or is there some other advantage to the PFILE:peeks? \$\endgroup\$ Nov 19 at 4:15
  • 1
    \$\begingroup\$ @luserdroog: There are circumstances under which this needs to unget more than one character, and the normal ungetc only guarantees the ability to unget one character. \$\endgroup\$ Nov 19 at 6:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.