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.
