Groovy hepcat
was jivin' on Wed, 20 Aug
2003 20:39:22 +0000 (UTC) in comp.lang.c.
Re: This blows my mind.'s a cool scene! Dig it!
>ext_u <> broke the eternal silence and spoke thus:
>
>> "Exercise 1-13 Write a program to print a histogram of the lengths of
>> words in its input. It is easy to draw the histogram with the bars
>> horizontal; a vertical orientation is more challenging."
>
>Because I'm bored, here is a probably very poor attempt...
Indeed it is.
>int main(int argc, char *argv[])
>{
> int i, j;
> int longest=0;
> int cur_length;
> int *lengths;
> int *tmp;
>
> if ( !(lengths=malloc(1)) ) exit(1);
>
> for(i=1;i < argc;i++) {
> if( (length=strlen(argv[i])) > longest ) {
> longest=length;
> if( (tmp=realloc(lengths,longest+1)) == NULL) exit(1);
> lengths=tmp;
> lengths[longest]=0;
> }
> lengths[length]++;
> }
>
> for(i=0;i < longest;i++) {
> for(j=0;j < lengths[i];j++)
> printf("#");
> printf("\n");
> }
>
> return 0;
>}
>
>Probably not what K&R (or sane people) would do...
Definitely not what people who know C would do.
> but it might work...
Then again, it might not. Let's examine the diagnostic output you
recieved when you tried compiling it, shall we? What's that? You
didn't try compiling it? Whell then, let's examine the diagnostic
output I recieved when I tried compiling it. (These diagnostic
messages are not unexpected.)
testing.c(9): Error! E1010: Type mismatch
Since you have not told the compiler that malloc() returns a pointer
to void (by including stdlib.h), it (if it conforms to C90) assumes
that malloc() returns an int. Hence the type mismatch when you attempt
to assign the return value to a pointer.
testing.c(12): Error! E1011: Symbol 'length' has not been declared
You apparently misspelled something here. I'm not sure what. Maybe
cur_length. (See below.)
testing.c(14): Error! E1010: Type mismatch
Since you have not told the compiler that realloc() returns a
pointer to void (eg., by including stdlib.h), it (if it conforms to
C90) assumes that realloc() returns an int. Hence the type mismatch
when you attempt to assign the return value to a pointer.
testing.c(14): Error! E1011: Symbol 'NULL' has not been declared
Since you have not declared NULL (eg., by including any of the
headers in which it is declared), it does not exist.
testing.c(5): Warning! W202: Symbol 'cur_length' has been defined, but
not referenced
This speaks for itself. You never used cur_length, though you
declared it. I think perhaps you misspelled it when you used it.
testing.c(14): Warning! W301: No prototype found for 'realloc'
testing.c(9): Warning! W301: No prototype found for 'exit'
testing.c(9): Warning! W301: No prototype found for 'malloc'
testing.c(12): Warning! W301: No prototype found for 'strlen'
testing.c(23): Warning! W301: No prototype found for 'printf'
These all speak for themselves. You have provided no declarations
for these functions (eg., by including the headers in which they are
declared).
Now, let's fix these problems. Insert the following lines at the
beginning of the program's source code:
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
Next, going on the assumption that "length" is supposed to be
"cur_length", change all occurrences of "length" to "cur_length".
Now my compiler happily compiles the code without a problem. But
that doesn't mean it's good code. There are still many things wrong
with it. Let's take a look at those now, shall we?
For a start, mainly as a matter of style, it is better to return
from main() rather than call exit(). But more importantly, you are
passing a non-portable value to exit(). The only values that can
portably be passed to exit() or returned from main() are 0,
EXIT_SUCCESS and EXIT_FAILURE (the latter two defined in stdlib.h).
Secondly (and this is a biggie), you are allocating one single,
solitary byte for a pointer to int. When you go to store or retrieve
an int in this allocated memory, the single byte may not be large
enough to hold such a beastie. Result: undefined behaviour. Sure, you
attempt to allocate more space later, but you never account for the
size of an int in your malloc() and realloc() calls. This is a very
serious problem.
You seem to be taking input from the command line instead of stdin.
I think the intention (of the exercise) is that input be taken from
stdin. (It's chapter 1 of K&R, remember. Command line arguments have
not been discussed yet.)
If the realloc() call fails, you're not releasing the original block
of memory. Thus, you have a possible memory leak on your hands.
Remember, if realloc() fails, the original block is untouched. That's
why you use an extra pointer when using realloc(), so you can still
access (and deallocate) the original memory block.
If cur_length is more than one greater than longest, then you need
to set *all* newly created elements of the reallocated array, not just
the end one, to a starting value (0). Otherwise, intervening values
will be indeterminate.
Why use printf() to inefficiently display a single character? Use
putchar() instead, and avoid printf()'s format parsing and variable
argument overhead.
--
Dig the even newer still, yet more improved, sig!
http://alphalink.com.au/~phaywood/
"Ain't I'm a dog?" - Ronny Self, Ain't I'm a Dog, written by G. Sherry & W. Walker.
I know it's not "technically correct" English; but since when was rock & roll "technically correct"?