A test for the C gurus

[Adam Parkin]

So I had to pick up some exam booklets yesterday from the CS
department office, and found accidentally mixed in a copy of an old
final exam from a Software Engineering course here at UVic. There was
one question on it that is bugging me a bit, and thought I'd throw it
up here to see what you all came up with:

--------
Figure 2 contains a fragment of C code. List and explain all of the
ways in which the code is wrong or could go wrong (possibly depending
on the nature of the input to the program). (Note: there are at least
7 distinct errors or defects in this short piece of code. There is no
credit at all for pointing out an error unless that error is
explained. Issues of coding style, such as poor choice for
identifiers, will not be counted as errors for the purposes of this
question)
----------

And then Figure 2 contained the following code (I added line numbers
for reference):

1 char Buffer[64];
2 char *line;
3
4 char *doSomething() {
5 char *s, *d;
6 char *line;
7 char done[2] = "OK";
8
9 scanf ("%s", Buffer); /* read a line from standard input */
10 line = malloc(64);
11 for (d=line, s=Buffer; *s != '\0'; s++, d++)
12 *d = *s;
13 printf ("Input string = %s\n", line);
14 free (Buffer);
15 return done;
16 }

I'm having trouble finding 7 distinct errors. I'm going to reply to
this post with what I find, but see what you come up with and let me
know what I'm missing. :)

Adam

[Paul Nienaber]

Should be char * doSomething(void) { (although this will be implied on
most modern compilers).

Line 7 is initializing a 2-character char array with a 3-character
constant, which is wrong.

Return value of malloc() is unchecked.

scanf's string-scanning is unbounded; Should be using fgets or somesuch.

Wrong pointer is passed to free() (buffer not line).

Return value of scanf is unchecked so we don't even know that it's
filled Buffer with a nul-terminated string.

%s does not read a *line*, per the comment; it reads a sequence of
non-whitespace characters.

~Paul

[Peter van Hardenberg]

this post with what I find, but see what you come up with and let me
know what I'm missing. :)

Adam

Okay, easy ones first. My C is rusty, and I'm refusing on principle to even try to compile this:

9: that string better not be more than 64 characters long
11: boy, i hope there's a \0 on the end of that input
14: free (Buffer) // not a pointer to allocated memory
15: nothing like returning a stack variable

--
Peter van Hardenberg
Victoria, BC, Canada
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut

[Adam Parkin]

Line 7: 2 elements isn't large enough to contain 'O', 'K', and the
null terminator
Line 9: potential buffer overrun
Line 10: return value of malloc isn't checked, thus line could be
null.
Line 11: the for loop condition only checks for the null terminator in
s, but not d (thus we could potentially go past the end of d)
Line 14: Buffer was not allocated via a call to malloc, thus it cannot
be free()'d

So what's #6 and #7?

Adam

[Paul Nienaber]

Ha, I missed yours from line 15.  Just kinda threw in the towel once I'd hit 7.  More correctly though, it's the address of a /block/ of memory on the stack, there's no addressof... :)

~p

[Adam Parkin]

Damn you guys are fast, I didn't even get my list up before the two of
you replied. :)

Missed the returning a local, I think that's definitely one that was
intended being one of the 7. So that's 6.

Not sure about the missing void from the function prototype though.
gcc will compile it without complaint, so I think it falls more under
the category of "poor style" rather than "error in the code".

I think the lack of checking the return value of scanf *might* be #7,
though honestly until I read Paul's post I didn't even realize scanf
returned a value (shows you how much I use that function). :p

Thanks,

Adam