Any way to take a word as input from stdin ?

[Ben Bacarisse]

Here is the outline which gives run time error of the free() call in
main():

This is not an outline for the design you explained. This is very
similar to what you posted before -- the get_input function prints the
words. I thought you wanted get_input to store them all in a dynamic
array and "return" that (return in quotes because the plan was for a
status code to be the actual return values -- the array would be a
char *** parameter).

char* pword; ....
free( &pword );

Whenever you see this you know it is wrong. &pword is the address of
a variable -- in this case a local object in main. You can't free
local variables. You can free what they point to (if they are
pointers) but that involves passing the value (i.e. the contents of
the variable) to free, not a pointer to the variable itself.

 

[arnuld]

This is not an outline for the design you explained. This is very
similar to what you posted before -- the get_input function prints the
words.

Thats just there unintentionally, for a memory check. I will remove it.

I thought you wanted get_input to store them all in a dynamic
array and "return" that (return in quotes because the plan was for a
status code to be the actual return values --

*Exactly* that..

the array would be a char *** parameter).

yes, but then get_single_word() will take a char*** as argument. When I try to
do that I get invalid pointer error. I have to admit that this busines of
pointers is really hell confusing. get_words() is already getting the address of
the pointer then why have to pass the address of the address of the
pointer to get_single_word(). Even when I do that my program compiles
fine but ends in doing semantic error. Program exits as soon as you input
one word.

2nd, someone told me on clc that if a function crosses 40 lines then
alarms should be ringing. get_single_word() is a 60 line function. Does
it need to be abstracted into 2 other functions ?

Whenever you see this you know it is wrong. &pword is the address of
a variable -- in this case a local object in main. You can't free
local variables. You can free what they point to (if they are
pointers) but that involves passing the value (i.e. the contents of
the variable) to free, not a pointer to the variable itself.

Oh.. no, I did not notice this stupidity .



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


enum { WORD_SIZE = 3, WORD_ERR = -1 };
enum { GSW_OK, GSW_ENOMEM, GSW_ENORESIZE } ;
enum { GI_OK = 0 , GI_ERR = WORD_ERR };


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



int main( void )
{
char* pword;
int input_err;
size_t words_count;

pword = NULL;
words_count = 0;
input_err = WORD_ERR; /* we will update the status in function call to get_input() */

input_err = get_words( &pword, &words_count );

if( WORD_ERR != input_err )
{
printf("words_count = %u\n", words_count);
}


/* call to - sort_input(...)
call to - printf_input(...) */

free( pword );


return 0;
}



int get_words( char** pword, size_t* w_cnt )
{
int err_catcher;
*w_cnt = 0;


while( (err_catcher = get_single_word(&pword)) && ( **pword != 0) )
{
if( GSW_OK == err_catcher )
{
++*w_cnt;
}
else
{
return GI_ERR;
}
}

return GI_OK;
}




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 == word_begin )
{
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++ )
{
new_mem = realloc( **ppc, (word_length_interval * word_length * sizeof *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;
}
======================== OUTPUT ================================
[arnuld@dune ztest]$ gcc -ansi -pedantic -Wall -Wextra sort-input.c
[arnuld@dune ztest]$ ./a.out
Like
words_count = 0
[arnuld@dune ztest]$

 

[arnuld]

stdin is a FILE*. What's the problem?

yes stdin is a FILE* but (ch = getchar()) is not. When user input EOF
(Ctrl-D), then ch has that value, how to check that ?



/* A program that will ask the user for input and then will sort the input alphabetically
and will print it on stdout

* As of now it does not sort any words but it will very soon. I am writing it in parts
* when parts work fine,then we put them in one place

* Since I did not want to put any limitation on input size in this program, I have used
* dynamic memory allocation to solve the problem. My intent in creating and then solving
* this problem was purely of C learning (as defined by ANSI standard) and nothing else. I
* wanted to learn C and hence this problem and quite heavy discussion of it on comp.lang.c.
* I owe many thanks to the brilliant help I got in comp.lang.c
*
* VERSION 1.0
*
*
*/


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


enum { WORD_SIZE = 3, WORD_ERR = -1 };
enum { GSW_OK, GSW_ENOMEM, GSW_ENORESIZE, GSW_EINPUT } ;
enum { GI_OK = 0 , GI_ERR = WORD_ERR };


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



int main( void )
{
char* pword;
int input_err;
size_t words_count;

pword = NULL;
words_count = 0;
input_err = WORD_ERR; /* we will update the status in function call to get_input() */

input_err = get_words( &pword, &words_count );

if( WORD_ERR != input_err )
{
printf("words_count = %u\n", words_count);
}


/* call to - sort_input(...)
call to - printf_input(...) */

free( pword );


return 0;
}



int get_words( char** pword, size_t* w_cnt )
{
int err_catcher;
*w_cnt = 0;


while( (err_catcher = get_single_word(&pword)) && ( **pword != 0) )
{
if( GSW_OK == err_catcher )
{
++*w_cnt;
}
else
{
return GI_ERR;
}
}

return GI_OK;
}




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 == word_begin )
{
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++ )
{
new_mem = realloc( **ppc, (word_length_interval * word_length * sizeof *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;
}


if( feof(ch) )
{
*word_begin = '\0';
}
else if( ferror(ch) )
{
return GSW_EINPUT;
}


return GSW_OK;
}


=========================== OUTPUT =======================================
[arnuld@dune ztest]$ gcc -ansi -pedantic -Wall -Wextra sort-input.c
sort-input.c: In function `get_single_word':
sort-input.c:147: warning: passing arg 1 of `feof' makes pointer from integer without a cast
sort-input.c:151: warning: passing arg 1 of `ferror' makes pointer from integer without a cast
[arnuld@dune ztest]$

 

[James Kuyper]

yes stdin is a FILE* but (ch = getchar()) is not. When user input EOF
(Ctrl-D), then ch has that value, how to check that ?

What does that have to do with it? what you pass to feof() and ferror()
is not the value returned by the I/O function. What you pass it is the
FILE* associated with the stream that you are using the I/O function on.
The getchar() function is exactly equivalent to getc(stdin); it's
provided as a separate function from getc() only for convenience. All
you need to do is check feof(stdin) and ferror(stdin).

 

[Ben Bacarisse]

Thats just there unintentionally, for a memory check. I will remove
it.

Please do, or mark it like this:

#ifdef DEBUG
....
#endif

so we know it is not what you are really trying to do.

*Exactly* that..
OK...


yes, but then get_single_word() will take a char*** as argument.

No. A function takes what it must due to its design.
get_single_word() needs a char ** because its job is to set a char *
that "does not belong to it". It is passed a pointer to the char *
that is must set: get_single_word(char **);

Giving get_input() a char *** parameter does not mean that this must be
passed to get_single_word. You get to say what you pass!

 

[Andrew Poelstra]

yes, but then get_single_word() will take a char*** as argument. When I try to
do that I get invalid pointer error. I have to admit that this busines of
pointers is really hell confusing. get_words() is already getting the address of
the pointer then why have to pass the address of the address of the
pointer to get_single_word(). Even when I do that my program compiles
fine but ends in doing semantic error. Program exits as soon as you input
one word.

Since get_single_word() needs to return a pointer to a string (char *),
and does so "by reference", its parameter should be a char **. No more
indirection is necessary.

2nd, someone told me on clc that if a function crosses 40 lines then
alarms should be ringing. get_single_word() is a 60 line function. Does
it need to be abstracted into 2 other functions ?

Probably. Your style uses more vertical space than most, though, so
any line-count-related rules of thumb need to be adjusted for that.
Input functions are usually very short, 10 or 15 lines plus whatever
validation is necessary.

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


enum { WORD_SIZE = 3, WORD_ERR = -1 };
enum { GSW_OK, GSW_ENOMEM, GSW_ENORESIZE } ;
enum { GI_OK = 0 , GI_ERR = WORD_ERR };


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



int main( void )
{
char* pword;
int input_err;
size_t words_count;

pword = NULL;
words_count = 0;
input_err = WORD_ERR; /* we will update the status in function
call to get_input() */

Do you not like initializing at declaration? It seems to me it would
be clearer to do so. (nb I wrapped your comment to fit in one line.)

input_err = get_words( &pword, &words_count );

if( WORD_ERR != input_err )
{
printf("words_count = %u\n", words_count);
}

size_t might be a different size than unsigned int, so you need to
cast it or use the %z specifier introduced in C99.

/* call to - sort_input(...)
call to - printf_input(...) */

free( pword );


return 0;
}

main() is fine other than those two minor nits.

int get_words( char** pword, size_t* w_cnt )

This seems suspicious - if get_words() needs to return a list of
strings, the required type is char ***. (Pointer to 'array' of
pointers to strings.)

{
int err_catcher;
*w_cnt = 0;


while( (err_catcher = get_single_word(&pword)) && ( **pword != 0) )

And then here you would pass *pword++, which gives get_single_word
a char ** to redirect to a string, and increments pword to point
to the next available item in the list.

{
if( GSW_OK == err_catcher )
{
++*w_cnt;
}
else
{
return GI_ERR;
}
}

return GI_OK;
}




int get_single_word( char*** ppc )

If you look, you find that you always refer to ppc with **ppc, which
suggests that a level of indirection can be removed.

{
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 == word_begin )
{
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++ )
{
new_mem = realloc( **ppc, (word_length_interval * word_length * sizeof *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;
}

This whole loop needs to be redone using my suggestions, as well as those of
other posters.

 

[arnuld]

Please do, or mark it like this:

#ifdef DEBUG
...
#endif

so we know it is not what you are really trying to do.

okay.

No. A function takes what it must due to its design.
get_single_word() needs a char ** because its job is to set a char *
that "does not belong to it". It is passed a pointer to the char *
that is must set: get_single_word(char **);

Giving get_input() a char *** parameter does not mean that this must be
passed to get_single_word. You get to say what you pass!

Thanks so much for the technical advice. I will take some time to write
some pseudo code again to get done with the design of the functions first.
I will make a new post with my new pseudo code and discussion of
function design.

 

[arnuld]

What does that have to do with it? what you pass to feof() and ferror()
is not the value returned by the I/O function. What you pass it is the
FILE* associated with the stream that you are using the I/O function on.
The getchar() function is exactly equivalent to getc(stdin); it's
provided as a separate function from getc() only for convenience. All
you need to do is check feof(stdin) and ferror(stdin).

I can't, that input character is needed by the program, hence I use ch to
store it. If i use feof(stdin) then I will loose the input and will get
only the integer vale returned by feof or ferror. The input character is
needed because we are storing them

 

[Nick Keighley]

I can't, that input character is needed by the program, hence I use ch to
store it. If i use feof(stdin) then I will loose the input and will get
only the integer vale returned by feof or ferror.

I'm baffled. Why will you "lose the input"? What does this mean.

The input character is
needed because we are storing them

what?


int read_char_and_check (FILE* f)
{
char ch;
ch = getc (f);
if (ch == EOF)
{
if (feof(f))
printf ("end-of-file\n");
else
printf ("error reading file\n");
}
return ch;
}

 

[James Kuyper]

I can't, that input character is needed by the program, hence I use ch to
store it. If i use feof(stdin) then I will loose the input and will get
only the integer vale returned by feof or ferror. The input character is
needed because we are storing them

What are you talking about? Just replace feof(ch) with feof(stdin);
there's no reason why doing so would require you to make any changes to
the line where you write ch=getchar().

 

[vippstar]

int read_char_and_check (FILE* f)
{
char ch;
ch = getc (f);

That's implementation defined, if char is signed and EOF < SCHAR_MIN
or if a value greater than SCHAR_MAX is returned.

if (ch == EOF)
{
if (feof(f))
printf ("end-of-file\n");
else
printf ("error reading file\n");

It doesn't have to be an error. Assuming char is unsigned, EOF == -1,
you'd get the same results for reading UCHAR_MAX and EOF.

}
return ch;
}

It's possible that this return value is never equal to EOF, even if ch
== EOF

 

[Nick Keighley]

I'm baffled. Why will you "lose the input"? What does this mean.


what?

int read_char_and_check (FILE* f)
{
    char ch;

int ch; /* doh! */

 

[Barry Schwarz]

I can't, that input character is needed by the program, hence I use ch to
store it. If i use feof(stdin) then I will loose the input and will get
only the integer vale returned by feof or ferror. The input character is
needed because we are storing them

You seem to think that feof or ferror perform some action against the
data in the stream. That is not true. They merely test and report
the status of certain indicators associated with the stream.

 

[Tim Rentsch]

Off topic, but what the hell. I, living in Southern California, have yet
to meet anybody who uses linux. Besides myself, and people I have given
computers to. I work in construction, and have been in many offices, and
have seen nothing but window and mac boxes. Eventually, if my children
let me have some free time, I will join a LUG.

Linux is alive and well in Pasadena and the San Gabriel Valley.
Check SGVLUG - in fact there is a meeting tomorrow night (the
second Thursday of each month), and drop-ins are welcome.