On Sat, 17 Apr 2004, Matt Mayers wrote:
I'm here to promote my C library project for writing CGI progs in C.
The project is called Stutter and can be found at
http://freshmeat.net/projects/stutter
I'm horrible, I know. =)
Not at all! We all love topical discussion of real-world standard C
programming! :)
% gcc -W -Wall -ansi -pedantic -O2 -c stutter.c
stutter.c: In function `getParam':
stutter.c:49: warning: implicit declaration of function `strcasecmp'
stutter.c:74: warning: char format, different type arg (arg 3)
% gcc -W -Wall -ansi -pedantic -O2 -c stutter_ex.c
stutter_ex.c:5: initializer element is not constant
stutter_ex.c:6: initializer element is not constant
stutter_ex.c:7: initializer element is not constant
All of gcc's criticisms are right on.
'strcasecmp' needs to be declared before you can call it, and defined
somewhere. More importantly, *you* can't define 'strcasecmp' *anywhere*,
because names beginning with 'str' and another letter are reserved for
the implementation's use only.
You have in "stutter.c" the line
sscanf(__pQueryString, "%s", &__qs);
That's wrong, because 'sscanf' is expecting a pointer to 'char' as its
third argument, and the value of the expression '&__qs' is a pointer
to array[MAX_QS_SIZE] of 'char'. More importantly, you can't use the
name '__qs' either, because the implementation *also* reserves names
beginning with two underscores for its own use. I presume you're worried
that if you use just plain old 'qs' as the identifier, the client code
will come up with naming conflicts. You should read [one of] the
section[s] on the 'static' keyword in a good book; 'static' will help
you make sure "package-local" variables like 'qs' don't escape.
In 'stutter_ex.c', you have some global variables you're trying to
initialize with the results of function calls. A little thought will
show you that you can't do that: when would those functions be called?
Before 'main'? *Nothing* in standard C happens before 'main'! So you
need to move their initializations --- in fact, their entire definitions
--- inside the body of 'main'.
Logic bug: Inside 'getParam', you're assigning the result of 'getenv'
to '__pQueryString', and then immediately calling 'getenv' again. This
will, on many systems, *overwrite* the old string's data, effectively
trashing the value of the string pointed to by '__pQueryString'. This
is a major bug, and needs fixing badly!
Silliness: That 'while' loop with the increment inside it is just a
silly way of writing '__qscount += strlen(__pQSDummy);'
Silliness: Your "check for overflow" inside 'getParam' is kind of
silly. It's really checking whether the user (the web interface)
entered an extra-long line; no "buffer overflow" in the usual sense of
the term will ever occur. Wouldn't it be better to go ahead and handle
long lines, since machine-generated queries often end up really long
anyway? Your users might thank you, and it's a great way to get
introduced to non-trivial uses for 'realloc'. (Also, your line length
is WAAAAY too long in those comments. 80 characters, /please!/)
Slight silliness: 'IntFromHex' is ASCII-specific. This might be
perfectly fine; I wouldn't be surprised if the relevant Internet
standards for CGI specified that all URL-thingies were supposed to
be encoded in ASCII. But you could have replaced the whole thing
with
unsigned hex;
sscanf(__pChars, "%2x", hex);
return hex;
and saved yourself some time and disk space.
I see that you used 'static' correctly on 'IntFromHex', so you should
have known better with all those other variables and functions.
The comment on 'printBasicHeader' shows that you're a little shaky
on what exactly the difference is between arrays and pointers. Google
"'Chris Torek' 'The Rule'".
Also, I've recently converted to the practice of using 'puts' instead
of 'printf' whenever possible. It's a religious issue, I think, but
I'll mention it anyway. One rationale is that 'puts' might shave a few
nanoseconds off your execution time, by bypassing all of 'printf''s
format-specifier interpreter code.
3 is not a portable argument to 'exit', and 'theSize' is never used
in file "stutter_ex.c".
Finally, an ease-of-use suggestion: You might consider adding a
function to the library that would make HTML forms output easier
on the eye. For example, the client could replace
printf("<form action=\"/cgi-bin/stutter_ex\" method=\"GET\">");
with something like
HTMLdump("<form action='/cgi-bin/stutter_ex' method='GET'>");
and then the 'HTMLdump' function would take care of replacing all
's with "s. You could even reuse some of your existing code! ;-)
HTH, and thanks for sharing,
-Arthur
