[cs631apue] HW#5 notes

[Jan Schaumann]

Hello,

There were a few things that may be worth mentioning, since several of
you encountered these issues:


(1) Looking at the Makefiles you all submitted, it seems to me that very
few of you currently fully understand when one needs to specify which
flags to the compiler.  Recall that compilation consists of a few
distinct steps, only the last of which involves linking object files
together with shared libraries into an executable.  Only at this stage
do the linker-relevant flags (such as "-l") have any effect.

Consider this example:

cc -Wall -lcrypto -c bfed.c -o bfed.o

Here you are telling the compiler to perform compilation only (via "-c",
ie no linking), to enable all warnings ("-Wall"), to place the ouput
into the file 'bfed.o' ("-o") and to link against the 'crypto' library
("-lcrypto").  Note how "-c" and "-lcrypto" conflict.

Secondly, recall that some flags to the compiler are position sensitive.
That is, they are processed in the order they are encountered, which may
yield unexpected results.  The "-l" flags are position sensitive, and
therefor are usually specified _after_ any other object files.

This becomes important when you have multiple libraries from different
locations.  Your compilation commands should thus be:

cc -Wall -c bfed.c -o bfed.o
cc -o bfed bfed.o -lcrypto

Please review these concepts and make sure you understand them.  (There
are many websites that will explain this in more detail.)


(2) Parsing command-line options is a solved problem.  Don't write your
own functions to do this, use getopt(3) or getopt_long(3).  This ensures
that the order in which flags are given does not matter, that arguments
can be specified in a manner that is consistent with the other unix
tools and will reduce the odds of you introducing any bugs
significantly.

Remember, any code you don't have to write is a win for you.

When accepting a flag that takes an argument, remember to specify the
letter followed by a colon in the getopt string, so that you do not have
to process the command-line arguments yourself to find the argument:

while ((ch = getopt(argc, argv, "dehk:")) != -1)


(3) Error messages are printed to stderr, without exception.  Errors
yield a non-zero exit status.

$ ./bfed -e -k invalidKey <data >out
$ echo $?
0
$ 

should not yield

$ cat out
'invalidKey' is not a valid key
$ 

Instead:

$ ./bfed -e -k invalidKey <data >out
'invalidKey' is not a valid key
$ echo $?
1
$ 


(4) malloc(3) can fail.  Please, believe me.  I don't quite know what
else to say, but it seems that many of you may do error checking for
some functions, but refuse to do so for malloc(3).  malloc(3) can fail.
And so you need to check its results before attempting to store data in
what you hope may be memory allocated to your process but what may also
well be NULL.

This does, of course, hold for all functions: if a function can fail,
your program needs to check its return and handle such failure
appropriately.


(5) Avoid code duplication.  Any time you write the same block of code
twice with only minor modifications, factor it out into a function and
call that instead.  Your code becomes more easy to maintain this way.

Also, don't call the same function with different arguments in different
code blocks when you can just make one such call.  Instead of:

if (condition) {
	call_function(arg1, arg2, 0);
} else {
	call_function(arg1, arg2, 1);
}

do this:

arg3 = 1;
if (condition) {
	arg3 = 0;
}
call_function(arg1, arg2, arg3);

This makes a difference when you later have to change the other
arguments -- now you only have one place where you change them.


(6) Don't mix spaces and tabs.  Inconsisten indentation makes your code
hard to read.  Any useful code editor can show you whether or not a
space is a regular space or a tab, and can convert from one to the other
automatically, even if you're pasting code you found on the internet
into your editor.


(7) To test if a character is a valid hex character, use isxdigit(3);
again, no need to write any code yourself.  For anything as common as
this, spend some time to look around the standard libraries on your
system before you start to write your own implementation.


-Jan