Re: [dev] print utility

From: Robert Ransom <>
Date: Sat, 30 Mar 2013 20:46:05 -0700

On 3/30/13, Calvin Morrison <> wrote:
> On 30 March 2013 22:30, Robert Ransom <> wrote:
>> On 3/30/13, Calvin Morrison <> wrote:
>>> What do you guys think of the tool? Of the code? It does one thing and
>>> one thing well.
>> Or perhaps you're just learning C and wanted someone to review your code.
> A bit of both really, a good code review is very nice for me to learn,
> and it has a purpose, not some silly lesson from a text-book without a
> real use. How many times can I say hello to the world?

m*n times, where m is the number of programming languages you've
started learning and n is the number of natural languages you've seen
a greeting in.

But I think I've written dumpargs.c (a test program that prints out
argc and argv[0..n]) more times than that.

>> Style issues:

>> * If you're writing a quick program that no one else will have to use
>> as a tool, positional arguments are acceptable, but the argument that
>> could best be generalized to zero or multiple instances should be last
>> on the command line if possible. (If you had put the filename second
>> instead of first, it would be much easier to fix the program to
>> support reading from stdin.)
> So would it be proper to handle it with a few if statements?
> if argc is 2 try to read standard input
> if argc is 3 read argv[2]
> if argc is 3+ or 1, print to stderr a usage statement

I prefer:

FILE *input = stdin;

if (argc < 2 || argc > 3) usage();

if (argc >= 2) {
  requested_line = parse_ulonglong(argv[1]);
  /* detect and handle whatever errors parse_ulonglong doesn't */
if (argc >= 3) {
  input = fopen(argv[2]);
  /* detect and handle errors */

> if a switch appropriate? else if?

switch is trickier to use correctly. else doesn't seem useful for
argument handling.

>> * If you're writing a quick program with at most one input stream and
>> at most one output stream, use stdin and stdout, and omit the
>> filename-handling code. (That's certainly faster.)
> I am having trouble with understanding this, do you mean something to
> the effect of this?
> cat print.c | print 2
> #include <stdio.h>

I prefer:

print 2 <print.c

>> * You're using both exit() and return in main. This is noticeably
>> inconsistent.
> I fixed this
> Which is preferable? I suppose in this example it does not matter
> because returning is the same as exit from the main function?

I prefer to use return in main(), and exit() only in functions where I
can't return an error indication (or don't want to).

>> * You're initializing line in its declaration, but then
>> re-initializing it before it is read from. That's silly -- omit the
>> first initialization.
> Should I have a seperate declaration and then intialization later or
> just combine the statements:
> int line = atoi(argv[2]);

This is bad in C89, because the declaration must come before you check
whether argc is at least 2. If you're using a language which allows
variable declarations anywhere within a block (e.g. C99, Go, or C++),
this is good if you can use it, but remember that a variable
declaration within an if statement will not be 'in scope' for the rest
of the function.

> or
> int line;
> line = atoi(argv[2]);

This would be ugly if the lines were adjacent.

>> * Your usage message should be a string literal in the fprintf or
>> fputs call that prints it, which should in turn be in a separate
>> usage() function. (Usually you'll need to print the usage message in
>> more than one case.)
>> Correctness issues:

>> * 'print foo bar baz' will (a) not complain that it was given more
>> than two arguments, and (b) choose an unspecified line, depending on
>> how your libc handles errors in atoi. (strtoul is better. Remember
>> to detect errors by checking errno.)
> I will look at strtoul, though I don't know anything about error
> checking.

Set errno to 0 before calling strtoul; if an error occurs, it will be
non-zero afterward. If your input string should only contain one
integer, also make sure **endptr == '\0' (but only after checking that
errno != 0).

> Can error checking be done with atoi?

Not reliably.

>> * LINE_MAX is a bug, not a feature. If you really can't be bothered
>> to support arbitrarily long lines, at least define your maximum line
>> length explicitly.

I forgot to mention the reason for this -- I've read that at least one
OS (probably the HURD) defines LINE_MAX as something unreasonably low
(100?) (but the system utilities are GNU, and can handle lines as
large as malloc will allow), and I didn't know what LINE_MAX actually
is on any of the OSes I use. (On my Linuxes, it turns out to be

Robert Ransom
Received on Sun Mar 31 2013 - 05:46:05 CEST

This archive was generated by hypermail 2.3.0 : Sun Mar 31 2013 - 05:48:06 CEST