Reading a string of unknown size

T

Tonio Cartonio

I have to read characters from stdin and save them in a string. The
problem is that I don't know how much characters will be read.

Francesco
 
R

Richard Heathfield

S

santosh

Tonio said:
I have to read characters from stdin and save them in a string. The
problem is that I don't know how much characters will be read.

You'll have to read the input, either character-at-a-time, (using
getc() or fgetc()), or line at a time, (using fgets()), and store them
into a block of memory which can be dynamically resized as you read in
more input.

It's fairly easy to do this yourself, but if you want to use an already
available one, try CBFalconer's ggets() function. Search the group's
archive. The URL is mentioned quite regularly.
 
S

Santosh

I have to read characters from stdin and save them in a string. The
problem is that I don't know how much characters will be read.

int main()
{
char *str = NULL, ch ;
int i = 0 ;
str = (char*) malloc (2*sizeof(char)) ;
*str = '\0' ;

while( (ch=getchar()) != '\n' )
{
*(str+i) = ch ;
i++ ;
str = (char*) realloc(str, (2*sizeof(char)) + i ) ;
}
*(str+i) = '\0' ;

printf("\n\n %s ", str) ;

getch() ;
return 0;
}


:)
 
R

Richard Heathfield

Santosh said:
int main()
{
char *str = NULL, ch ;
int i = 0 ;
str = (char*) malloc (2*sizeof(char)) ;

Here is your first bug.
*str = '\0' ;

Here's the second.
while( (ch=getchar()) != '\n' )

Here's the third.
{
*(str+i) = ch ;
i++ ;
str = (char*) realloc(str, (2*sizeof(char)) + i ) ;

Here's the fourth and fifth, at least.
}
*(str+i) = '\0' ;

printf("\n\n %s ", str) ;

Here's your sixth.
getch() ;

And your seventh.
return 0;
}


:)

In fourteen lines of code (excluding spaces and braces), you managed at
least seven bugs. What are you smiling about?
 
S

Santosh

:)In fourteen lines of code (excluding spaces and braces), you managed at
least seven bugs. What are you smiling about?

The program works correctly.

No offence pal, but make sure you are at least 10% right before you
reply to any post.
 
R

Richard Heathfield

Santosh said:
The program works correctly.

Really? Let's explore that, shall we?

foo.c:2: warning: function declaration isn't a prototype
foo.c: In function `main':
foo.c:3: `NULL' undeclared (first use in this function)
foo.c:3: (Each undeclared identifier is reported only once
foo.c:3: for each function it appears in.)
foo.c:5: warning: implicit declaration of function `malloc'
foo.c:5: warning: cast does not match function type
foo.c:8: warning: implicit declaration of function `getchar'
foo.c:12: warning: implicit declaration of function `realloc'
foo.c:12: warning: cast does not match function type
foo.c:16: warning: implicit declaration of function `printf'
foo.c:18: warning: implicit declaration of function `getch'
make: *** [foo.o] Error 1

Oh, look - it doesn't even compile.
No offence pal, but make sure you are at least 10% right before you
reply to any post.

I was 100% right that your program was a good 50% wrong (in terms of bugs
per line). And it doesn't compile on my system. If it compiles on yours
without generating at least one diagnostic message, then your compiler is
broken.

Furthermore, the OP did not indicate his platform (nor was there any need
for him to do that), let alone his implementation, so you can't just claim
"it works on *my* system", since there is no indication whatsoever that the
OP's system is the same as your system.
 
S

santosh

First include necessary headers: stdio.h, stdlib.h
int main()

Better yet, replace above with int main(void)
{
char *str = NULL, ch ;
int i = 0 ;
str = (char*) malloc (2*sizeof(char)) ;

Don't cast return value of malloc() in C. It can hide the non-inclusion
of it's prototype, (by way of failure to include stdlib.h), and, on
some implementations, can result in nasty crashes during runtime.

Since sizeof(char) is by definition 1, you can omit that and instead do
'2 * sizeof *str'. This has the advantage of becoming automatically
updated when you later on happen to change the type of *str.
*str = '\0' ;

And now you're possibly writing to a random area of memory, since you
failed to check the return value of malloc() above for failure.
while( (ch=getchar()) != '\n' )

Check for EOF not newline. Moreover getchar() returns an int value
which you're storing in a char variable, probably getting a spurious
garbage return value when end-of-file is encountered.
{
*(str+i) = ch ;

You've overwritten your earlier nul character.
i++ ;
str = (char*) realloc(str, (2*sizeof(char)) + i ) ;

Again, _don't_ cast the return value of XXalloc() functions in C, and
check the call for failure before proceeding further. Also change
sizeof(char) to sizeof *str.

Anyway, your allocation strategy is very inefficient. Your calling
realloc() once every iteration of the loop. This could result in
fragmentation of the C library's memory pool. Why not allocate in terms
of fixed sized or dynamically growing blocks, say 128 bytes or so to
start with?
}
*(str+i) = '\0' ;

printf("\n\n %s ", str) ;

Unless you terminate the output with a newline character, it's not
guaranteed to show up on screen, or wherever stdout happens to be
directed to.
getch() ;

Non-standard, unportable and unnecessary function. Just get rid of it.
 
S

Santosh

per line). And it doesn't compile on my system. If it compiles on yours
without generating at least one diagnostic message, then your compiler is
broken.

Furthermore, the OP did not indicate his platform (nor was there any need
for him to do that), let alone his implementation, so you can't just claim
"it works on *my* system", since there is no indication whatsoever that the
OP's system is the same as your system.

Do not get too excited and offensive, you forgot to include the
following files:
#include <stdio.h>
#include <conio.h>

If you are using a Unix system:
#include <stdio.h>
#include <sys/types.h> OR #include <system.h>
 
S

santosh

Santosh said:
The program works correctly.

No offence pal, but make sure you are at least 10% right before you
reply to any post.

Huh!?

It failes to even compile on my Linux system.

After culling the proprietary getch() call it compiles with several
warnings and runs sucessfully by sheer luck, making several unwarrented
assumptions, which though may hold now, are prone to disastrous failure
anytime, especially if the target machine is anything other than a
standard 32 bit PC.

Looks like you should apply your last statement above to yourself,
before rushing to dismiss others, who may possibly be more knowledgeble
on C than you.
 
R

Richard Heathfield

Santosh said:
Do not get too excited and offensive,

I am neither excited nor offensive.
you forgot to include the
following files:
#include <stdio.h>
#include <conio.h>

No, I didn't forget: you did.

And C offers no header named said:
If you are using a Unix system:
#include <stdio.h>
#include <sys/types.h> OR #include <system.h>

Wrong. That wouldn't help your program compile.

Furthermore, even with the inclusion of those headers, your program is still
broken in several important ways.

I suggest you stop defending and start thinking.

The Other Santosh has given a good analysis of some of the problems with
your program. If you don't understand that analysis, start asking
intelligent questions instead of acting all defensively.
 
S

Santosh

Anyway, your allocation strategy is very inefficient. Your calling
realloc() once every iteration of the loop. This could result in
fragmentation of the C library's memory pool. Why not allocate in terms
of fixed sized or dynamically growing blocks, say 128 bytes or so to
start with?

Very True, I agree with you.
I just wanted to give "Tonio Cartonio" the idea of how to do it.
 
R

Richard Heathfield

Santosh said:
Very True, I agree with you.
I just wanted to give "Tonio Cartonio" the idea of how to do it.

What you actually gave him was a very good demonstration of how not to do
it.
 
S

Spiros Bousbouras

Is Santosh a common name ? Or was Santosh
trying to impersonate santosh ? Personally
I didn't realize that Santosh was different than
santosh until santosh's first post and in fact I
was surprised at the way he was conducting himeslf.
(Before I realized they were different people that is.)
 
R

Richard Heathfield

Spiros Bousbouras said:
Is Santosh a common name ?

I couldn't say, but I once worked with someone of that name, a few years
ago. I doubt whether it's particularly rare.

<snip>
 
K

Keith Thompson

Richard Heathfield said:
Spiros Bousbouras said:

I couldn't say, but I once worked with someone of that name, a few years
ago. I doubt whether it's particularly rare.

It seems to be fairly common, judging by the results of a Google
search.

I suggest that one or both of the [Ss]antosh's currently posting here
consider using their full name to avoid confusion. (Neither of them,
of course, is obligated to follow my advice.)
 
S

Santosh Nayak

I suggest that one or both of the [Ss]antosh's currently posting here
consider using their full name to avoid confusion. (Neither of them,
of course, is obligated to follow my advice.)

Sounds Logical !
 
S

Sundar

santosh said:
First include necessary headers: stdio.h, stdlib.h


Better yet, replace above with int main(void)


Don't cast return value of malloc() in C. It can hide the non-inclusion
of it's prototype, (by way of failure to include stdlib.h), and, on
some implementations, can result in nasty crashes during runtime.

Since sizeof(char) is by definition 1, you can omit that and instead do
'2 * sizeof *str'. This has the advantage of becoming automatically
updated when you later on happen to change the type of *str.


And now you're possibly writing to a random area of memory, since you
failed to check the return value of malloc() above for failure.


Check for EOF not newline. Moreover getchar() returns an int value
which you're storing in a char variable, probably getting a spurious
garbage return value when end-of-file is encountered.


You've overwritten your earlier nul character.


Again, _don't_ cast the return value of XXalloc() functions in C, and
check the call for failure before proceeding further. Also change
sizeof(char) to sizeof *str.

Anyway, your allocation strategy is very inefficient. Your calling
realloc() once every iteration of the loop. This could result in
fragmentation of the C library's memory pool. Why not allocate in terms
of fixed sized or dynamically growing blocks, say 128 bytes or so to
start with?


Unless you terminate the output with a newline character, it's not
guaranteed to show up on screen, or wherever stdout happens to be
directed to.


Non-standard, unportable and unnecessary function. Just get rid of it.


Can u please further explain the following statement....

"Don't cast return value of malloc() in C. It can hide the
non-inclusion
of it's prototype, (by way of failure to include stdlib.h), and, on
some implementations, can result in nasty crashes during runtime."

I am unable to understand the intricacies of the above statement.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,906
Latest member
SkinfixSkintag

Latest Threads

Top