gets() function generates strong warning message with gcc compiler

P

paytam

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);

}
 
P

p_cricket_guy

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
 
S

spibou

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
 
W

websnarf

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.
 
A

apoelstra

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).)
 
S

Simon Biber

1) Proper malloc form is "p = malloc (n * sizeof *p);"
2) sizeof (char) is always one, so this is superfluous.
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);

Must also check for EOF:
while ((ch = getchar()) != '\n' && ch != EOF)

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]
 
C

CBFalconer

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>
 

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,743
Messages
2,569,478
Members
44,899
Latest member
RodneyMcAu

Latest Threads

Top