The Worst C Example Ever May 21, 2007

Posted by Imran Ghory in Software development

The Example:

strcpy(str, getpass("Enter password: "));

The first time I saw this I was shocked at how many problems this tiny example of code has (there’s a list of problems further on – if you want to try and figure out the problems yourself try to write a working getpass() function based upon this caller).

To put this example in context it came from Question 6.5 of the comp.lang.c FAQ in which this example is given as an example for how to achieve what the following piece of code is trying to do:

extern char *getpass();

char str[10];

str = getpass("Enter password: ");

For those of you who aren’t C programmers the above chunk of code won’t work as getpass() is returning a pointer to an array (in C strings are represented as arrays of chars) and C doens’t support implicit array copying.

However the “fixed” example is almost as bad as the original, only made marginally better by the fact it technically works. However it is horrendous as an example of good coding practice.

So what’s wrong with it ?

The newbie programmer has almost certainly implemented getpass() using the following method:

char* getpass(void)

{

char password[10];

scanf("%s

", password);

return password;

}

This reads the password into a local variable which it then returns a pointer to it. The worst thing about this is the code will actually work sometimes and not other times leaving the programmer in a terrible confusion (local variables are allocated on the stack hence will get overwritten after the function ends).

A more spohisticated way to write this function and how it would be generally be done would be to use dynamically allocated memory as in the following:

char* getpass(void)

{

char* password = (char*) malloc(10);

scanf("%9s

", password);

return password;

}

However in the original bad example the code is throwing away the pointer which getpass() returns, which means that it’s not possible to free that memory. Hence you end up with a memory leak, not good. So this solution is not good in the context.

The best possible getpass() you could get away with is to declare a static char array variable which you return a pointer to

char* getpass(void)

{

static char password[10];

scanf("%9s

", password);

return password;

}

However this makes the function non-thread-safe and non-renterant which limits its use, and as well as that you end up storing the password in memory for the permanent life of the program.

So as you can see it’s impossible for even a good programmer to write a good getpass() function that can be called by strcpy(str, getpass(“Enter password: “)) , let alone the beginners that this document is targeted at.

A bonus bug

Plus for bonus points, there’s yet another sutble bug in this code. It uses strcpy() in an unsafe manner. Even professional developers often slip up when using strcpy, generally professionals always want to use strlcpy as strcpy is almost never used correctly. The reason it’s bad is the following scenario, imagine getpass() was changed to be the following:

char* getpass(void)

{

static char password[100];

scanf("%99s

", password);

return password;

}



What makes it unsafe is that strcpy doesn’t know how many characters to copy, hence it just keeps copying from the source to the destination until it runs out of characters. Now that getpass() returns more than ten characters what happens ? – if you’re lucky the program will crash – if you’re unlucky the person typing in the password can take over the program and bypass the password protection.

So What ?

Well hopefully the above has convinced you that if you’re writing for beginners you should try to write examples that not only work, but also show good development practices. Especially online where space isn’t as major a consideration as it is in books.