input word

[Chad]

Using debugger means shutting down your brain. Thats what I think because
If I can't understand what I wrote in just 24 lines, I better throw the
code and start again.

I really don't see how a debugger can shut down your brain. I always
use a debugger at work.

 

[CBFalconer]

I really don't see how a debugger can shut down your brain. I
always use a debugger at work.

Well?

 

[CBFalconer]

CBFalconer said:

the objective of keeping the source of a routine small enough to
fit on a single CRT screen.

I can usually easily read code I wrote thirty years ago, because I
have adhered to this principle [1]. Forty year old code is another
matter, because I hadn't yet learned the key facts. Forty-five is
even worse. The age matters, but not the language.

Presumably, then, fggets is forty years old (and pre-dates the C
language). Here is one CRTful of fggets:

int fggets(char* *ln, FILE *f)
{
int cursize, ch, ix;
char *buffer, *temp;

*ln = NULL; /* default */
if (NULL == (buffer = malloc(INITSIZE))) return NOMEM;
cursize = INITSIZE;

ix = 0;
while ((EOF != (ch = getc(f))) && ('\n' != ch)) {
if (ix >= (cursize - 1)) { /* extend buffer */
cursize += DELTASIZE;
if (NULL == (temp = realloc(buffer, (size_t)cursize))) {
/* ran out of memory, return partial line */
buffer[ix] = '\0';
*ln = buffer;
return NOMEM;
}
buffer = temp;
}
buffer[ix++] = ch;
}

Kinda sawed-off, isn't it?

I don't think you do anyone any favors by republishing my code with
lines snipped off. Maybe I am slow, but it took me a comparison
with the real source to realize what you were doing.

For the benefit of other readers, here are the terminal missing
lines:

}
if ((EOF == ch) && (0 == ix)) {
free(buffer);
return EOF;
}

buffer[ix] = '\0';
if (NULL == (temp = realloc(buffer, (size_t)ix + 1))) {
*ln = buffer; /* without reducing it */
}
else *ln = temp;
return OK;
} /* fggets */
/* End of ggets.c */

 

[Ian Collins]

CBFalconer said:

CBFalconer said:

<snip>

the objective of keeping the source of a routine small enough to
fit on a single CRT screen.

I can usually easily read code I wrote thirty years ago, because I
have adhered to this principle [1]. Forty year old code is another
matter, because I hadn't yet learned the key facts. Forty-five is
even worse. The age matters, but not the language.
Presumably, then, fggets is forty years old (and pre-dates the C
language). Here is one CRTful of fggets:
[incomplete rendition of fggets snipped]
Kinda sawed-off, isn't it?

I don't think you do anyone any favors by republishing my code with
lines snipped off.

I was simply illustrating that your claim - "keeping the source of a
routine small enough to fit on a single CRT screen" - is not matched by
your code.

He might gave a large CRT...

 

[CBFalconer]

CBFalconer said:

Now THAT was NOT NICE!

Anyone that leaves such a gaping opportunity requires it.

 

[CBFalconer]

.... snip ...

Note that CBFalconer's fggets function was not in fact excessively
long. That was not the point I was making about it. Rather, I was
making the point that an arbitrary rule such as "no more than one
screenful" doesn't really work.

And the 24 line screen was incidental. I was actually pushing the
rule of seven, which I think you can see if you go back and read.

 

[arnuld]

Yes, 40 lines is too long.

okay, forget about general function writing procedure. Don't you think the
while loop (with memory re-allocation) in my function is placed in an odd
place. To me, it feels like, this whole while loop should not be in this
function, or I am just getting confused (emotionally ?):


int get_single_word( char** ppc )
{
unsigned ele_num;
int ch;
size_t word_length, word_length_interval;
char* word_begin;
char* new_mem;

ele_num = 0;

word_length = WORD_SIZE;
word_length_interval = 2;

*ppc = malloc(word_length * sizeof(**ppc));
word_begin = *ppc;


if( NULL == ppc )
{
return GSW_ENOMEM;

}


while( (EOF != (ch = getchar())) && isspace(ch) )
{
continue; /* trailing whitespace */
}


if( EOF != ch )
{
*word_begin++ = ch;
ele_num = 1;
}


while( (EOF != (ch = getchar())) && (! isspace(ch)) )
{
if( (word_length - 1) == ele_num++ )
{
new_mem = realloc( *ppc, (word_length_interval * word_length * sizeof **ppc) );
*ppc = new_mem;

if( new_mem )
{
word_begin = new_mem + (word_begin - *ppc);
word_length *= word_length_interval;
*ppc = new_mem;
}
else
{
*word_begin = '\0';
return GSW_ENORESIZE;
}
}

*word_begin++ = ch;
}


*word_begin = '\0';

return GSW_OK;
}

 

[arnuld]

You use isspace to detect whitespace. isspace will return true for
several other characters besides these three.

I mean whatever whitespace there is, newlines, tabs etc etc. I will change
the comments to reflect this.

Surely this has been mentioned before. ppc contains the address of
pword in main. It can never be NULL. You probably meant *ppc.

Oh.. my .. I reall don't understand why the program was working with this
bug.

And I know your excessive vertical white space has been discussed.
Vertical white space is a matter of style but, to be blunt, yours sucks.

:-\

between function definitions: 2 vertical tabs

between variable assignments and variable 1 vertical tab
initializations:

point where variable assignments and/or 2 vertical tabs
initializations end and other expressions
start:

just before final return statement: 2 vertical tabs



Thats my style. is there something wrong with it ?

While neither condition applies in this case, for practice you could
attempt to compartmentalize some aspect of g_s_w. As an example, you
could move the 16 lines of code that deal with reallocating the space
*ppc points to into a separate function. It would require only three
arguments (the current value of *ppc, the address of word length, and
the address of new_mem).

I tried it but i get dangling pointer errors. See below for code.

They are mutually exclusive in this case so you only need to use one or
the other. If the one you use returns 1, then that condition is true.
(E.g., if you use feof and it returns 1, then you reached end of file
and don't have an error). If the one you use returns 0, then the other
condition is true.

/* a function written in ANSI C, that takes a word from stdin and manages the memory dynamically
*
* Since there is no generally agreed definition of what a 'word' is. I have taken a very simple
* approach:
*
* A word is a contiguous collection of non-whitespace characters. A whitespace is defined
* by isspace().
*
*/


#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>


enum { WORD_SIZE = 2 };
enum { GSW_OK, GSW_ENOMEM, GSW_ENORESIZE } ;


int get_single_word( char** );
int alloc_new_mem( char** , char**, char**, size_t* );


int main( void )
{
char* pword;


while( (GSW_OK == get_single_word(&pword)) && ( *pword != 0) )
{
printf("You entered: [%s]\n", pword);
free( pword );
}


return 0;
}




int get_single_word( char** ppc )
{
unsigned ele_num = 0;
int ch = EOF;
size_t word_length = WORD_SIZE;
char* word_begin = NULL;
char* new_mem = NULL;
size_t* pwl = &word_length;

*ppc = malloc(word_length * sizeof(**ppc));
word_begin = *ppc;


if( NULL == *ppc )
{
return GSW_ENOMEM;

}


while( (EOF != (ch = getchar())) && isspace(ch) )
{
continue; /* Leading whitespace */
}


if( EOF != ch )
{
*word_begin++ = ch;
ele_num = 1;
}


while( (EOF != (ch = getchar())) && (! isspace(ch)) )
{
if( (word_length - 1) == ele_num++ )
{
if( GSW_OK != alloc_new_mem( ppc, &word_begin, &new_mem, pwl ) )
{
return GSW_ENORESIZE;
}
}

*word_begin++ = ch;
}

*word_begin = '\0';

return GSW_OK;
}



int alloc_new_mem( char** ppc, char** word_begin, char** new_mem, size_t* pwl )
{
size_t word_length_interval = 2;

*new_mem = realloc( *ppc, (word_length_interval * WORD_SIZE * sizeof **new_mem) );

if( *new_mem )
{
*word_begin = *new_mem + (*word_begin - *ppc);
*pwl *= word_length_interval;
*ppc = *new_mem;
}
else
{
**word_begin = '\0';
return GSW_ENORESIZE;
}

return GSW_OK;
}




========================= OUTPUT =====================
[arnuld@dune ztest]$ gcc -ansi -pedantic -Wall -Wextra get-single-word.c
[arnuld@dune ztest]$ ./a.out
Love
You entered: [Love]
Looooooooooooooooooooooooooooooooooooooooooooooooooove
*** glibc detected *** realloc(): invalid next size: 0x09068008 ***
Aborted
[arnuld@dune ztest]

 

[Richard Bos]

Another glimpse into CBF's stone-age existence. How many clc'ers really
still use a CRT screen, I wonder?

I. They're a lot less blocky than most TFTs - and yes, I _can_ see the
difference, even if most people can't.

Richard

 

[Nick Keighley]

I. They're a lot less blocky than most TFTs - and yes, I _can_ see the
difference, even if most people can't.

until recently I had dual monitors one flat screen one CRT.
Some of my screen backgrounds were noticibly different on the two
screen and the "blockiness" of the flat screen was apparent.

I know people who still using CRTs (it costs money to
replace them).

 

[James Kuyper]

:-\

between function definitions: 2 vertical tabs

between variable assignments and variable 1 vertical tab
initializations:

point where variable assignments and/or 2 vertical tabs
initializations end and other expressions
start:

just before final return statement: 2 vertical tabs

I suspect that you mean "blank line" rather than "vertical tab". I've
never deliberately used a vertical tab, and I have no idea whether I've
ever seen them, and I've heard that there are many displays and printers
that don't implement them. However, if I understand the relevant
wikipedia article correctly, just as a horizontal tab causes a jump to
the next horizontal tab stop (traditionally, but not necessarily 8
spaces apart), a vertical tab causes a jump to the next vertical tab
stop (traditionally but not necessarily 6 lines apart).

Thats my style. is there something wrong with it ?

Yes, by spreading things out so much, it makes it harder to see
different parts of your program at the same time, whether looking at a
screen display or a printed page. If you really were using the number of
vertical tabs that you specify, the problem would be much worse.

 

[arnuld]

I suspect that you mean "blank line" rather than "vertical tab".

yes.


Yes, by spreading things out so much, it makes it harder to see
different parts of your program at the same time, whether looking at a
screen display or a printed page. If you really were using the number of
vertical tabs that you specify, the problem would be much worse.

I thought vertical tab and blank lines were synonyms. I use blank-lines.

 

[arnuld]

Another glimpse into CBF's stone-age existence. How many clc'ers really
still use a CRT screen, I wonder?

<OT>
me, all the time. I have yet to see a friend of mine who uses TFT. TFT is
very expansive.
</OT>

 

[Richard Bos]

me, all the time. I have yet to see a friend of mine who uses TFT. TFT is
very expansive.

I've never seen one growing larger, but they can be quite expensive.

Richard

 

[arnuld]

However, I'll buy it as "development standard" in your shop (assuming
that this is code written for a company, and the code-writing group has
standards wrt function body length).

Nope. This code was written by myself, for myself, only to learn about the
dynamic memory allocation and pointer business.

1) Reduce statements to their most compact form
For example, the 5 line
if ( NULL == ppc )
{
return GSW_ENOMEM;


would become the one line
if (NULL == ppc ) return GSW_ENOMEM;
eliminating four lines,

I will like to see what other clc posters has to say about this say. If
they agree, I will drop this kind into my style-bucket. Personally, I
liked it

and the 4 line
while( (EOF != (ch = getchar())) && isspace(ch) )
{
continue; /* Leading whitespace */
}
would lose three lines to become
while( (EOF != (ch = getchar())) && isspace(ch) ) ;

Same as earlier.

2) Combine commonalities into single elements. The two while() loops
have some common test elements; you could combine the two loops
into
one, and distinguish the differences within the loop body

3) Create new functions from old logic. Your get_single_word() function
works in four phases:
1 - sanity check,
2 - skip leading whitespace,
3 - empty word handler,
4 - word string handler
Break one or more of these out into it's own function, and modify
get_single_word() to utilize the new function instead of its own
inline code. A good candidate might be the word string handler logic.

Hm.. thta the logic I was looking for.

...SNIP...
From what I can see, your main() only looks for a
GSW_OK; /all/ other returns cause a silent program termination.

is that worse ?

Lew
(posted from google groups due to failure with my
regular nntp service)

Thats why I did not see this post earlier. I got it when I unblocked the
google groups.


Thanks

 

[Nick Keighley]

please leave attributions in

}


I will like to see what other clc posters has to say about this say. If
they agree, I will drop this kind into my style-bucket. Personally, I
liked it

if you like it and it's reasonably clear and you don't have any
coding standards you have to comply with then stick with it!

I'd code it like this:

if (NULL == ppc)
return GSW_ENOMEM;

At least one clc regular never ommits the brackets and
would do something similar to you. The reasoning is that
adding another line to the while body is error prone
and you could forget to add the brackets.

At least one fairly regular poster would dislike the one
line version as he would view it as debugger hostile.


I'd definitly have the ";" on a different line as
I find the single line version *very* easy to misread


<snip>

 

[James Kuyper]

arnuld wrote:
....

I will like to see what other clc posters has to say about this say. If
they agree, I will drop this kind into my style-bucket. Personally, I
liked it

The compact form presents problems due to the fact that the if condition
and the return statement are on the same line. The most serious one is
that in many (most?) debuggers, it's only possible to set a single break
point on a line. If you want to set a break point on the return
statement, you're forced to put a break point of then entire if
statement. The result is that the break point will trigger every single
time the if() is executed, regardless of whether or not the return
statement is executed.

The second problem is that compilers will identify the location of a bug
by the line number. It sometimes happens that the same diagnostic
message could be referring to either the 'if' condition, or the return
statement; putting them on separate lines will make it easier to figure
out where the actual error is.

I therefore always put each statement on a separate line. But this is a
matter of programming style, there are arguments for other approaches.

 

[Ben Bacarisse]

okay, forget about general function writing procedure. Don't you think the
while loop (with memory re-allocation) in my function is placed in an odd
place. To me, it feels like, this whole while loop should not be in this
function, or I am just getting confused (emotionally ?):

You can write it that way, but not because "the while loop should not
be in this function" but because you can find a part (the includes the
while loop) that makes sense as a separate function.

For another example, when you've finished, you will find that in two
places you need to change the size of an allocated piece of memory. I
often write the code to do this (and to track the new size) as a
function because I know I may need it in more than one place. That is
not the only reason to write a function, but it is a good one.

On another topic you state your style is to:

| between function definitions: 2 vertical tabs
|
| between variable assignments and variable 1 vertical tab
| initializations:

I think you mean between declarations and first assignment.

| point where variable assignments and/or 2 vertical tabs
| initializations end and other expressions
| start:
|
| just before final return statement: 2 vertical tabs

I would not do this, but as I have said before, style is not worth
arguing about. However, you don't follow your own rules. In the
code below, I have left all blank lines quoted -- I usually omit them
when they come next to my added remarks.

int get_single_word( char** ppc )
{
unsigned ele_num;
int ch;
size_t word_length, word_length_interval;
char* word_begin;
char* new_mem;

ele_num = 0;

word_length = WORD_SIZE;
word_length_interval = 2;

*ppc = malloc(word_length * sizeof(**ppc));
word_begin = *ppc;

One one of the blank lines above is accounted for by your rules.

if( NULL == ppc )
{
return GSW_ENOMEM;

}

and the one before the }?

while( (EOF != (ch = getchar())) && isspace(ch) )
{
continue; /* trailing whitespace */
}


if( EOF != ch )
{
*word_begin++ = ch;
ele_num = 1;
}

Here you have three pairs of blank lines. What rules accounts for
these?

while( (EOF != (ch = getchar())) && (! isspace(ch)) )
{
if( (word_length - 1) == ele_num++ )
{
new_mem = realloc( *ppc, (word_length_interval * word_length * sizeof **ppc) );
*ppc = new_mem;

Here there is only one. I can't see why.

if( new_mem )
{
word_begin = new_mem + (word_begin - *ppc);
word_length *= word_length_interval;
*ppc = new_mem;
}
else
{
*word_begin = '\0';
return GSW_ENORESIZE;
}
}

Which rule governs this space?

*word_begin++ = ch;
}

and again which rules make you put two here...

*word_begin = '\0';

and only one here when your rule is for two before the final return?

 

[CBFalconer]

Lew Pitcher wrote:
.... snip ...


I will like to see what other clc posters has to say about this
say. If they agree, I will drop this kind into my style-bucket.
Personally, I liked it

That one depends on your debugging practices. If you want to be
able to trap after the decision, but before executing the return,
you want a separate line, such as:

if (!ppc)
return GSW_ENOMEM;

and note that you don't need the "NULL =" portion at all.

I very rarely use a debugger, so I would write it as:

if (!ppc) return GSW_ENOMEM;

 

[CBFalconer]

.... snip ...

please leave attributions in


I'd definitly have the ";" on a different line as
I find the single line version *very* easy to misread

Which is why the 'continue' is useful here, as in:

while ((EOF != (ch = getchar()))} && isspace(ch)) continue;

Note the extra (), which to me solidifies what is tested.