Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Re: This blows my mind.

Reply
Thread Tools

Re: This blows my mind.

 
 
ataru@nospam.cyberspace.org
Guest
Posts: n/a
 
      08-20-2003
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...

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... but it might work...

--
Christopher Benson-Manica | Jumonji giri, for honour.
ataru(at)cyberspace.org |
 
Reply With Quote
 
 
 
 
ataru@nospam.cyberspace.org
Guest
Posts: n/a
 
      08-20-2003
broke the eternal silence and spoke thus:
> Because I'm bored, here is a probably very poor attempt...


Talk about poor It's nice to have a compiler...

#include <stdio.h>

int main(int argc, char * argv[])
{
int i, j;
int longest = 0;
int cur; /* damn variable names */
int * lengths; /* holds the lengths for later */
int * tmp;

if( !(lengths=(int *)malloc(1)) ) exit(1);

for( i = 1; i < argc; i++ )
{
if( (cur = strlen(argv[i])) > longest)
{
longest = cur;
if( !(tmp = (int *)realloc(lengths, longest + 1)) ) exit(1);
lengths = tmp;
lengths[longest] = 0;
}
lengths[cur]++;
}

for( i = 0; i <= longest; i++ )
{
printf("%d ", i);
for( j = 0; j < lengths[i]; j++ )
printf("#");
printf("\n");
}

return 0;
}

[17:47:14][asuka@soryuu:~]> ./a.out a a a ab a ac aaa aa aaa aaaa aa b a c
0
1 #######
2 ###
3 ###
4 #

--
Christopher Benson-Manica | Jumonji giri, for honour.
ataru(at)cyberspace.org |
 
Reply With Quote
 
 
 
 
Mike Wahler
Guest
Posts: n/a
 
      08-20-2003
<> wrote in message
news:bi0qk0$hfu$...
> broke the eternal silence and spoke thus:
> > Because I'm bored, here is a probably very poor attempt...

>
> Talk about poor It's nice to have a compiler...


Your code is still very poor, and imo will harm
rather than help the OP. Your compiler might have helped
you a bit, but you told it to shut up by casting
the return values of 'malloc()' and 'realloc()'.

>
> #include <stdio.h>
>
> int main(int argc, char * argv[])
> {
> int i, j;
> int longest = 0;
> int cur; /* damn variable names */
> int * lengths; /* holds the lengths for later */


This is a pointer. It cannot represent a 'length',
only a memory address.

> int * tmp;
>
> if( !(lengths=(int *)malloc(1)) ) exit(1);


You have failed to include the header which declares
'malloc()'. This results in your incorrect casting
of its return value to hide the fact that it will
return an int, very probably corrupting the valid
address it tried to return.

And why allocate a single byte?

If 'malloc()' fails, you abruptly terminate the
program, giving the user no indication of what's
wrong.

The only standard, portable values for the argument
to 'exit()' are the value zero, or one of the
macros 'EXIT_SUCCESS' or 'EXIT_FAILURE'.

>
> for( i = 1; i < argc; i++ )
> {
> if( (cur = strlen(argv[i])) > longest)


You have failed to include the header which declares
'strlen()'. This again can corrupt the return value
(which btw is type 'size_t', not 'int', as your
failure to include the prototype will force the
compiler to assume.

> {
> longest = cur;
> if( !(tmp = (int *)realloc(lengths, longest + 1)) ) exit(1);


Same problems here as with 'malloc()'.

IMO it's poor practice to put the statement controlled by
an 'if' on the same line. Use another, indented line.
Much more readable.

> lengths = tmp;
> lengths[longest] = 0;
> }
> lengths[cur]++;
> }
>
> for( i = 0; i <= longest; i++ )
> {
> printf("%d ", i);
> for( j = 0; j < lengths[i]; j++ )
> printf("#");
> printf("\n");
> }
>
> return 0;
> }
>
> [17:47:14][asuka@soryuu:~]> ./a.out a a a ab a ac aaa aa aaa aaaa aa b a c
> 0
> 1 #######
> 2 ###
> 3 ###
> 4 #


After reading OP's post, do you really think he'll
understand any of this? He's on chapter ONE, for
goodness sake! Do you think he knows what e.g.
'malloc()' is?

Your code is horribly broken anyway. I suggest you
do a bit of reading in K&R yourself.

-Mike




 
Reply With Quote
 
ataru@nospam.cyberspace.org
Guest
Posts: n/a
 
      08-20-2003
Mike Wahler <> broke the eternal silence and spoke thus:

> Your code is still very poor, and imo will harm
> rather than help the OP. Your compiler might have helped
> you a bit, but you told it to shut up by casting
> the return values of 'malloc()' and 'realloc()'.


Well, had I been smart and not hurrying I would have compiled -Wall..

> And why allocate a single byte?


Um, because I was in a hurry, and I'm dumb...?

malloc(sizeof(int));
malloc((length + 1) * sizeof(int));

> You have failed to include the header which declares
> 'strlen()'. This again can corrupt the return value
> (which btw is type 'size_t', not 'int', as your
> failure to include the prototype will force the
> compiler to assume.


Isn't defaulting to int not part of the standard anymore? If it still is,
why?

> IMO it's poor practice to put the statement controlled by
> an 'if' on the same line. Use another, indented line.
> Much more readable.


> After reading OP's post, do you really think he'll
> understand any of this? He's on chapter ONE, for
> goodness sake! Do you think he knows what e.g.
> 'malloc()' is?


I know he doesn't... I'm really looking forward to someone posting a "real"
solution, because I just don't know how K&R could possibly want it done w/o
malloc...

> Your code is horribly broken anyway. I suggest you
> do a bit of reading in K&R yourself.


Indeed... (although I really do know a *bit* more than it seems, really...)

--
Christopher Benson-Manica | Jumonji giri, for honour.
ataru(at)cyberspace.org |
 
Reply With Quote
 
ataru@nospam.cyberspace.org
Guest
Posts: n/a
 
      08-20-2003
Mike Wahler <> broke the eternal silence and spoke thus:

> Why sizeof (int)? The program will read characters.
> And why the +1? The program will not be creating
> strings.


I don't know.... I should really just abandon this thread, and post under a
new name, so I might have a chance of not being taken for a moron...

> No, it's not. But if you're coding by the new standard,
> invoking undeclared functions is not allowed. Ya can't
> have it both ways.


Well, I *meant* to declare them, does that count for anything?? I really
would have liked the code not to compile, rather than ending up being
(justly!) chastised for my stupidity here...

> It's not. But many (if not most) folks are still using
> C89 translators, since there doesn't seem to be much
> impetus to widely implement C99, for whatever reasons.


So, why does the program work at all? If I haven't declared these functions,
how can calling them have any meaning? Why did such dreadful code have the
indecency to appear to work fine? (don't say just to humiliate me lol)

> I hope they don't. Handing out ready-made solutions helps
> the learner not at all. Better he post his attempts, and
> get help and corrections. That way he'll learn something.
> Remember this is a learning exercise. I suspect they
> intend the reader to simply define some arbitrary-size
> array, e.g.


> char text[1000];


I guess... so next time I will a) do it the easy way, and b) not post my
"solution" here unless I somehow regain some shreds of dignity

> I certainly hope so. But whether using C89 or C99, it's
> still *always* wrong to cast the return value from
> 'malloc()' or 'realloc()'.


I *knew* something was fishy when I got warnings about pointer-integer
conversion, but being stubborn, I was like "take THAT gcc!" without pondering
that maybe the dumbass in the situation was me

> I hope I didn't offend you too much by 'ripping into'
> your code like that. I just couldn't let it be.


Oh no, it was totally justified!! I'm just utterly ashamed that after several
years of coding C (and liking it!) that I could conceivably come up with code
so dreadfully broken, and have the hubris to actually think it worked...

--
Christopher Benson-Manica | Jumonji giri, for honour.
ataru(at)cyberspace.org |
 
Reply With Quote
 
ataru@nospam.cyberspace.org
Guest
Posts: n/a
 
      08-21-2003
Mike Wahler <> broke the eternal silence and spoke thus:

> Why sizeof (int)? The program will read characters.
> And why the +1? The program will not be creating
> strings.


Oh, I remember now - I'm storing the *lengths* of the strings in a dynamic
array, so I can print them later. So sizeof(int) is right in that case, isn't
it?

--
Christopher Benson-Manica | Jumonji giri, for honour.
ataru(at)cyberspace.org |
 
Reply With Quote
 
Roger Willcocks
Guest
Posts: n/a
 
      08-21-2003

<> wrote in message
news:bi0sbo$hjv$...
....
> ... I'm really looking forward to someone posting a "real"
> solution, because I just don't know how K&R could possibly want it done

w/o
> malloc...

....
> --
> Christopher Benson-Manica | Jumonji giri, for honour.
> ataru(at)cyberspace.org |


--snip--

#include <stdio.h>

#define MAXLENGTH 10

int main(int argc, char* argv[])
{
int c, length, i;
int histogram[MAXLENGTH];
int toolong;

toolong = 0;
for (length = 0; length < MAXLENGTH; length++)
histogram[length] = 0;

length = 0;
while ((c = getchar()) != EOF) {
if (c == ' ' || c == '\t' || c == '\n' || c == '\r') {
if (length > 0) {
length--;
if (length < MAXLENGTH)
histogram[length]++;
else
toolong++;
length = 0;
}
}
else {
length++;
}
}

for (length = 0; length < MAXLENGTH; length++) {
printf("%d: ", length + 1);
for (i = 0; i < histogram[length]; i++)
printf("#");
printf("\n");
}

printf("%d words were too long\n", toolong);

return 0;
}
--snip--

--
Roger


 
Reply With Quote
 
Peter Shaggy Haywood
Guest
Posts: n/a
 
      08-23-2003
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"?
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Netframework 1.1 install blows up Tom Beretvas ASP .Net 3 08-12-2005 09:53 PM
Firefox window blows up to more than screen size covering all Jim Firefox 5 06-01-2005 03:01 AM
Saving Excel document blows up Infragistics menu in frame Amy B ASP .Net 2 02-25-2005 08:37 PM
Re: Standby/Hibernate blows out wireless connection =?Utf-8?B?VGlt?= Wireless Networking 0 08-07-2004 09:55 PM
I.E. blows after running my app Gary ASP .Net 1 04-03-2004 05:01 PM



Advertisments
 



1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57