468,512 Members | 1,390 Online
Bytes | Developer Community
New Post

Home Posts Topics Members FAQ

Post your question to a community of 468,512 developers. It's quick & easy.

gets() function generates strong warning message with gcc compiler

hi all

can you tell me what's the wrong with this code?
I use gcc compiler,but when I wanted to use

gets() function in my code but it takes a dangerous warning(the gets
function is dangerous

and should not be used),I don't know why.Any way,I decided to write my
own gets()

function,but now it take segmentation fault error.Please help me
thanks

THIS IS THE CODE:

#include <stdio.h>

#include <string.h>

#include <stdlib.h>
char * getstring()

{

int k=1,j=0,i;

char ch;

char *source=malloc (sizeof(char)*k);

if(!source)

{

printf("\n\nCan not allocate sufficent memory\n\n");

exit(1);

}

else{

while((ch=getchar())!='\n')

{

if(j==k)
{

realloc(source,(k*=2));

if(!source)
{

printf("\ncan not allocat sufficiant memory\n");

exit(1);

}//end if

}//end if

*(source+j)=ch;

j++;
}//end while

*(source+j)='\0';

//Here it shows correctly

printf("\n\n\n\n\nThis is what you typed :%s\n",source);
}//end of else

return source;

}

void main()

{

char *st;

printf("Enter a string: ");

/*If you want to see the warning gets message uncomment here

gets(st);

*/
//I forgot here or next command generate the segmentation fault
strcpy(st,getstring());

printf("\n\n\n\n\nThis is what you typed :%s\n",st);

}

Aug 3 '06 #1
10 5902
pa****@gmail.com wrote:
hi all

can you tell me what's the wrong with this code?
I use gcc compiler,but when I wanted to use

gets() function in my code but it takes a dangerous warning(the gets
function is dangerous

and should not be used),I don't know why.Any way,I decided to write my
own gets()

function,but now it take segmentation fault error.Please help me
thanks

THIS IS THE CODE:

#include <stdio.h>

#include <string.h>

#include <stdlib.h>
char * getstring()

{

int k=1,j=0,i;

char ch;

char *source=malloc (sizeof(char)*k);

if(!source)

{

printf("\n\nCan not allocate sufficent memory\n\n");

exit(1);

}

else{

while((ch=getchar())!='\n')

{

if(j==k)
{

realloc(source,(k*=2));

if(!source)
{

printf("\ncan not allocat sufficiant memory\n");

exit(1);

}//end if

}//end if

*(source+j)=ch;

j++;
}//end while

*(source+j)='\0';

//Here it shows correctly

printf("\n\n\n\n\nThis is what you typed :%s\n",source);
}//end of else

return source;

}

void main()

{

char *st;

printf("Enter a string: ");

/*If you want to see the warning gets message uncomment here

gets(st);

*/
//I forgot here or next command generate the segmentation fault
strcpy(st,getstring());
BOOM *?!@?!?!
st is pointing nowhere here. You need to either allocate memory
to st or just point st to whatever getstring() returns.
[st = getstring();]
There are potential problems in getstring() too. I will leave it for
the experts to comment on.

-p_cricket_guy
>
printf("\n\n\n\n\nThis is what you typed :%s\n",st);

}
Aug 3 '06 #2
pa****@gmail.com wrote:
hi all

can you tell me what's the wrong with this code?
I use gcc compiler,but when I wanted to use

gets() function in my code but it takes a dangerous warning(the gets
function is dangerous

and should not be used),I don't know why.
Wikipedia is your friend ;-)
Any way,I decided to write my
own gets()

function,but now it take segmentation fault error.Please help me
thanks

THIS IS THE CODE:

#include <stdio.h>

#include <string.h>

#include <stdlib.h>
char * getstring()

{

int k=1,j=0,i;

char ch;

char *source=malloc (sizeof(char)*k);

if(!source)

{

printf("\n\nCan not allocate sufficent memory\n\n");

exit(1);

}

else{

while((ch=getchar())!='\n')

{

if(j==k)
{

realloc(source,(k*=2));

if(!source)
{

printf("\ncan not allocat sufficiant memory\n");

exit(1);

}//end if

}//end if

*(source+j)=ch;

j++;
}//end while

*(source+j)='\0';

//Here it shows correctly

printf("\n\n\n\n\nThis is what you typed :%s\n",source);
}//end of else

return source;

}

void main()

{

char *st;

printf("Enter a string: ");

/*If you want to see the warning gets message uncomment here

gets(st);

*/
//I forgot here or next command generate the segmentation fault
strcpy(st,getstring());

printf("\n\n\n\n\nThis is what you typed :%s\n",st);

}
But you're not using your own function , you're
still using gets() !

Some remarks about your function:

1. int k=1,j=0,i;

Since it is almost certain that the input string will
be more than 1 characters long it would save some
calls to malloc if you initialized k to a larger value ,
100 say. Or your function may accept an additional
parameter which will specify the initial buffer size.

2. while((ch=getchar())!='\n')

You also need to check for EOF.

3. *(source+j)='\0';

For all you know the last execution of the loop may have
completely filled the buffer so this will write past the end.
Spiros Bousbouras

Aug 3 '06 #3
sp****@gmail.com wrote:
pa****@gmail.com wrote:
while((ch=getchar())!='\n')

You also need to check for EOF.
So you would need to declare ch as int rather than
char.

Aug 3 '06 #4

p_***********@yahoo.co.in wrote:
pa****@gmail.com wrote:
hi all
thanks alot.i forgot that st is a pointer.

Aug 3 '06 #5
pa****@gmail.com wrote:
can you tell me what's the wrong with this code?
I use gcc compiler,but when I wanted to use gets() function in my
code but it takes a dangerous warning(the gets function is dangerous
and should not be used), I don't know why.
Topicallity aside, you should ask this question in comp.std.c. They
don't believe or acknowledge the existance of this question in
programmers. It would be good for them to hear it.
[...] Any way, I decided to write my own gets()
Welcome to our coven. :) (Here's mine:
http://www.pobox.com/~qed/userInput.html)
#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char * getstring() {
int k=1,j=0,i;
char ch;
char *source=malloc (sizeof(char)*k);

if(!source) {
printf("\n\nCan not allocate sufficent memory\n\n");
exit(1);
}
else{
while((ch=getchar())!='\n')
{
if(j==k)
{
The following snippet is problematic:
realloc(source,(k*=2));
if(!source)
{
printf("\ncan not allocat sufficiant memory\n");
exit(1);
}//end if
You should replace it with:

char * tmp = realloc (source, k *= 2);
if (!tmp) {
puts ("\ncan not allocate sufficient memory");
free (source);
exit (1);
}
source = tmp;

Basically really *returns* with the reallocated pointer without
changing your source pointer. If successful your source pointer
becomes (potentially) invalid and the returned pointer has both the
contents and additional space for your input.
}//end if
*(source+j)=ch;
j++;
}//end while
*(source+j)='\0';
//Here it shows correctly
printf("\n\n\n\n\nThis is what you typed :%s\n",source);
}//end of else
return source;
}

void main() {
char *st;
printf("Enter a string: ");
/*If you want to see the warning gets message uncomment here
gets(st);
*/
//I forgot here or next command generate the segmentation fault
strcpy(st,getstring());
This is a memory leak. In this case it doesn't matter, but in general
you should capture the output of getstring() and at some point call
free() on it.

--
Paul Hsieh
http://www.pobox.com/~qed/
http://bstring.sf.net/

Aug 3 '06 #6
pa****@gmail.com wrote:
and should not be used),I don't know why.Any way,I decided to write my
own gets()
You might find Chuck Falconer's ggets() useful and/or instructive:

http://cbfalconer.home.att.net/download/ggets.zip

--
C. Benson Manica | I *should* know what I'm talking about - if I
cbmanica(at)gmail.com | don't, I need to know. Flames welcome.
Aug 3 '06 #7
On 2006-08-03, pa****@gmail.com <pa****@gmail.comwrote:
hi all

can you tell me what's the wrong with this code?
I use gcc compiler,but when I wanted to use

gets() function in my code but it takes a dangerous warning(the gets
function is dangerous

and should not be used),I don't know why.Any way,I decided to write my
own gets()
You answered your own question in your code: gets() /doesn't/ allocate
more memory when it runs out of space, so if the input is too long, it
will spill over into who-knows-where, resulting in Undefined Behavior.
function,but now it take segmentation fault error.Please help me
thanks

THIS IS THE CODE:
(If you don't mind, I'm going to remove the vertical spacing. It's hard
to read on a 80x24 screen. I'll also fix the erratic horizonal spacing.
Never, ever use tabs on Usenet. They rarely display correctly.)
>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char * getstring()
{
int k=1, j=0, i;
char ch;
char *source=malloc (sizeof(char)*k);
1) Proper malloc form is "p = malloc (n * sizeof *p);"
2) sizeof (char) is always one, so this is superfluous.
if(!source)
{
printf("\n\nCan not allocate sufficent memory\n\n");
exit(1);
}
Excellent.
else {
while ((ch = getchar()) != '\n')
{
if(j==k)
{
realloc(source,(k*=2));
if(!source)
{
printf("\ncan not allocat sufficiant memory\n");
exit(1);
}//end if
}//end if
1) In small functions like this, there's no need for those comments.
2) Since // comments aren't supported by C89, it's recommended that
you shy away from them until C99 is more popular. Also, they are
not recommended on Usenet, because line wrapping breaks them.
3) realloc() doesn't change the value of source: you'll have no idea
whether it changes. You need to capture the return value.
4) In this case, you probably want to try to allocate less memory
if realloc() fails; k doesn't have to be so big. You'll need to
use a temporary pointer for this:
tmp = realloc (p, size);
if (tmp)
p = tmp;
else
{
size = size * 3 / 2;
/* Repeat, using a while(!tmp) loop instead of an if statement. */
}

5) You may want to write a function that checks a pointer and kills
the program on NULL:

void check_mem (void *p)
{
if (p == NULL)
{
puts ("Out of memory!");
exit (EXIT_FAILURE); /* exit(1) isn't portable to some systems. */
}
}
*(source+j)=ch;
j++;
Why not just do:
*source++ = ch;
++j;

That's a more well-understood idiom.
}//end while
*(source+j)='\0';
Then here, just do
*source = 0;
//Here it shows correctly
printf("\n\n\n\n\nThis is what you typed :%s\n",source);
Most of those newlines aren't necessary. (Of course, putting printf()
statements into code that will likely become part of a library is a
bad idea anyway, and will probably be removed when you're done testing.)
} //end of else
return source;
}

void main()
main() returns an int:
int main(void)
{
char *st;
printf("Enter a string: ");
/*If you want to see the warning gets message uncomment here
gets(st);
*/
You'll get a lot more than an error message. You might want to grab
a good C book for more details.
//I forgot here or next command generate the segmentation fault
strcpy(st,getstring());
Of course! You haven't allocated any memory in st. You need to have
space before you can use strcpy() to put stuff in that space. In this
case, you shouldn't use strcpy() at all; just do
st = getstring();
(Remember, char* isn't actually a string; it's a pointer to char(s).)
printf("\n\n\n\n\nThis is what you typed :%s\n",st);
}
--
Andrew Poelstra <http://www.wpsoftware.net/projects>
To reach me by email, use `apoelstra' at the above domain.
"Do BOTH ends of the cable need to be plugged in?" -Anon.
Aug 3 '06 #8
ap*******@localhost.localdomain wrote:
On 2006-08-03, pa****@gmail.com <pa****@gmail.comwrote:
>#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char * getstring()
{
int k=1, j=0, i;
char ch;
char *source=malloc (sizeof(char)*k);

1) Proper malloc form is "p = malloc (n * sizeof *p);"
2) sizeof (char) is always one, so this is superfluous.
>if(!source)
{
printf("\n\nCan not allocate sufficent memory\n\n");
exit(1);
}
Excellent.
Good but not excellent. Error messages generally should go to stderr
rather than stdout.
fprintf(stderr, "Cannot allocate sufficient memory\n");

The argument 1 to the exit function has no defined meaning across
various C implementations. On some it will indicate success, on some it
will indicate failure, and on some it will indicate some other meaning
-- perhaps "security breach - kill all user programs now". It has an
unknown meaning and could even be dangerous. You are much better off
using the standard form: exit(EXIT_FAILURE);
>else {
while ((ch = getchar()) != '\n')
Must also check for EOF:
while ((ch = getchar()) != '\n' && ch != EOF)
> {
if(j==k)
{
realloc(source,(k*=2));
if(!source)
You fundamentally misunderstand how function calls work in C if you
think that realloc could have magically changed the value of 'source'.
Arguments to functions are passed a _copy_ of the _value_ of the argument.

realloc will return a pointer to the new memory block. The old pointer
becomes invalid. You are not even allowed to read source's value to
check whether it is now a null pointer.

[rest of code snipped]

--
Simon.
Aug 4 '06 #9
we******@gmail.com wrote:
pa****@gmail.com wrote:
>can you tell me what's the wrong with this code?
I use gcc compiler,but when I wanted to use gets() function in my
code but it takes a dangerous warning(the gets function is
dangerous and should not be used), I don't know why.

Topicallity aside, you should ask this question in comp.std.c.
They don't believe or acknowledge the existance of this question in
programmers. It would be good for them to hear it.
>[...] Any way, I decided to write my own gets()

Welcome to our coven. :) (Here's mine:
http://www.pobox.com/~qed/userInput.html)
And my version, designed for the simplicity of use of gets with
absolute safety, is available at:

<http://cbfalconer.home.att.net/download/ggets.zip>

--
Chuck F (cb********@yahoo.com) (cb********@maineline.net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.netUSE maineline address!
Aug 5 '06 #10

pa****@gmail.com wrote:
hi all

can you tell me what's the wrong with this code?
I use gcc compiler,but when I wanted to use

gets() function in my code but it takes a dangerous warning(the gets
function is dangerous
Please read this man page for information about why gets() should be
avoided:

http://www.openbsd.org/cgi-bin/man.c...86&format=html

Thanks,

Brian

Aug 5 '06 #11

This discussion thread is closed

Replies have been disabled for this discussion.

Similar topics

32 posts views Thread by Marcus | last post: by
2 posts views Thread by Alfonso Morra | last post: by
302 posts views Thread by Lee | last post: by
12 posts views Thread by Bill Pursell | last post: by
280 posts views Thread by jacob navia | last post: by
16 posts views Thread by pereges | last post: by
By using this site, you agree to our Privacy Policy and Terms of Use.