You didn't give us much of a hint as to what you think "doesn't work"
means. But one obvious problem is the code above. Once you pass a
pointer to free, you can't then try to dereference it. Try something
like
tptr = ptr->next;
free(ptr);
ptr = tptr;
(with an appropriate declaration of tptr, of course.)
Regards,
-=Dave
Sorry, I am not a mind reader, you will have to spell out what exactly
your issue is.
> why?
What is your input, do you have enough memory allocated for 'nms'?
******************************************
i have change code as your code,it is my fault;
now i find it throw error at the first " scanf("%d
%s",&ags,nms); //-------------------here
"
when i input "10 finy", it throw a error.
okay now let's see the '10' gets stored in ags and 'finy' which needs
5 (4 bytes for "finy" and 1 extra byte for '\0') bytes of storage
space, you try to store it in 'nms' which has how much space ... ?
(Ans. 1 byte)
oh!my god! yes,you are right!
thanks!
> char *nms=" ";
nms doesn't have any storage that he's allowed to write to. It points
to a string literal.
See: http://c-faq.com/decl/strlitinit.html
John
The declaration of nms was
char *nms=" ";
so nms points to *two* bytes (' ' and '\0'). But, as someone else
already pointed out, you're not allowed to write to those bytes,
because they're part of a string literal. (You *might* get away with
it, but it's undefined behavior.)
Another comment: white space can make your code easier to read.
Rather than
char *nms=" ";
try:
char *nms = " ";
And so on.
--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
/* BEGIN new.c */
#include <stdio.h>
#include <stdlib.h>
#define LENGTH 19
struct student {
int age;
char *nms;
struct student *next;
};
struct student *create(void)
{
struct student *head = NULL;
struct student *tail = NULL;
struct student *p = NULL;
int ags;
char *nms;
do {
p = malloc(sizeof *p);
if (p == NULL) {
puts("p == NULL");
exit(EXIT_FAILURE);
}
p -> next = NULL;
nms = malloc(LENGTH + 1);
if (nms == NULL) {
puts("nms == NULL");
exit(EXIT_FAILURE);
}
if (scanf("%d%s", &ags, nms) != 2) {
puts("scanf(\"%d%s\", &ags, nms) != 2");
exit(EXIT_FAILURE);
}
p -> age = ags;
p -> nms = nms;
if (head == NULL) {
head = p;
} else {
tail -> next = p;
}
tail = p;
} while (ags != 0);
return head;
}
int main(void)
{
struct student *next;
struct student *ptr = create();
while (ptr != NULL) {
next = ptr -> next;
printf("%d====%s\n", ptr -> age, ptr -> nms);
free(ptr -> nms);
free(ptr);
ptr = next;
}
return 0;
}
/* END new.c */
--
pete
**************************************
you say "you're not allowed to write to those bytes, because they're
part of a string literal"
but i try it,i can write bytes like follow:
char *nms=" ";
scanf("%s",nms);
you can try it.
The standard says that the behavior is undefined if you attempt
to modify such a string. It is possible that *you* can do that
but that's not something you should rely on.
--
Ioan - Ciprian Tandau
tandau _at_ freeshell _dot_ org (hope it's not too late)
(... and that it still works...)
Please trim quoted material. In particular, don't quote signatures
unless you're actually commenting on them.
> **************************************
> you say "you're not allowed to write to those bytes, because they're
> part of a string literal"
>
> but i try it,i can write bytes like follow:
>
> char *nms=" ";
> scanf("%s",nms);
>
> you can try it.
As I write above:
You *might* get away with it, but it's undefined behavior.
The most likely results of attempting to modify a string literal are
(a) your program immediately crashes, or (b) it "works", and the
string literal is modified.
Undefined behavior doesn't mean that the system is going to stop you
from doing it. It means the behavior is undefined; anything can
happen, so it's entirely up to you to avoid doing it.
See question 1.32 in the comp.lang.c FAQ, <http://www.c-faq.com/>.
--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
Let me try it... This is what happens when I run the program and type
'b':
$ foo
b
Segmentation fault
So it's not exactly working for me. :)
In addition to what others are saying about the behavior in this
instance being undefined, the standard says that string literals may be
in read-only memory, and can actually be shared in memory, for instance:
#include <stdio.h>
int main(void)
{
char *a = "foobar";
char *b = "foobar";
char *c = "frobozz";
printf("a = %p\n", a);
printf("b = %p\n", b);
printf("c = %p\n", c);
return 0;
}
prints the following on my system:
a = 0x8048524
b = 0x8048524
c = 0x804852b
(Note a and b point to the same place.)
So, on my system, if it were allowed, modifying a's "foobar" would
effectively also modify b's; it's probably not what was desired.
-Beej
>#include "stdio.h"
That should be:
#include <stdio.h>
>#include "malloc.h"
No such header. It should be:
#include <stdlib.h>
>struct student{
> int age;
> char *nms;
> struct student *next;
>};
>struct student *create(){
> int ags=0,size=sizeof(struct student);
> char *nms=" ";
Here you create a string literal of one character length, resulting
in two bytes being allocated (one for your single character, and one
for the terminating '\0'). This may be placed in memory marked as
being readable but not writable. String literals are not modifiable.
> struct student *head=NULL,*tail=NULL,*p=NULL;
> scanf("%d%s",&ags,nms); //-------------------here!!!!!!
And here you attempt to read an arbitrarily long string (likely more
than one character) into the non-modifiable, one character long string
literal, thus attempting to not only modify a non-modifiable object,
but actually overflow that, writing to memory you don't own; possibly
memory that doesn't even exist! BANG! And your program crashes. Or it
goes on "working", but ends up doing weird things to your hard drive.
Or it makes demons fly out of your nose.
Had you read the FAQ list of this newsgroup or simply lurked here
for a while you would know that scanf() is not the best thing to use
for interactive input. It is customary, upon first entering a
newsgroup, to read a month or two of articles (lurk) before posting
and to seek out and read the newsgroup's FAQ, if it has one. Failing
to do so before posting is very rude! Please read the FAQ
(http://www.eskimo.com/~scs/C-faq/top.html) before posting any more.
What is the user supposed to be enterring anyhow? A prompt would be
helpful.
> while(ags!=0){
> p=(struct student * )malloc(size);
Don't cast the return from malloc(). Do check the return from
malloc().
> nms=(char *)malloc(20);
Don't cast the return from malloc(). Do check the return from
malloc().
> p->age=ags;
> p->nms=nms;
Here you copy the address of your string literal to your struct
member. Thus, your struct member will point to your string literal. No
other storage has been set aside, and no data copied.
> p->next=NULL;
> if(head==NULL){
> head=p;
> }else{
> tail->next=p;
A bit dodgy! I think it will probably work, since the else clause is
not executed first time around (because head == NULL first time
through the loop). But it doesn't look good to dereference tail above
the point where you actually set it to a useful value..., assuming p
actually does contain a useful value.
> }
> tail=p;
> scanf("%d%s",&ags,nms);
And again you attempt to write X characters (where X is some unknown
number) to a one character long string literal.
> }
> return head;
>}
>int main(void) {
> struct student *ptr=create();
Check the return value. It's always a good idea.
> while(ptr){
> printf("%d====%s\n",ptr->age,ptr->nms);
> free(ptr->nms);
And here you're trying to free memory that was not allocated by
malloc(), calloc() or realloc(). BANG! More undefined behaviour! More
nasal demons! Remember, the nms member of your structure points at a
string literal.
> free(ptr);
> ptr=ptr->next;
And once again, BANG! You're dereferencing ptr after freeing the
memory it points at.
> }
return 0;
>}
>************************************
>i think that is no problem with it.
Think again! There are many problems here. There may be some I
haven't spotted; after all, I only gave your code a cursory look. Even
so, I still found many serous errors.
> but it doesn't work!
>why?
Because it's so severely broken.
--
Dig the even newer still, yet more improved, sig!
http://alphalink.com.au/~phaywood/
"Ain't I'm a dog?" - Ronny Self, Ain't I'm a Dog, written by G. Sherry & W. Walker.
I know it's not "technically correct" English; but since when was rock & roll "technically correct"?
thank a lot!
thanks for your excellent explaining!
There was no need to repeat the whole thing.
Sometimes I do that as a cheap and easy way of saving
the explanation in my email "Sent box".
--
pete
That's all very well for you, but it is anti-social to then continue to
post it to a newsgroup. All good mail- and news-readers have features to
save a post without sending it. The saving is not the problem; the
unsnipped sending is.
Richard
Hey, I have a patent on that. Cease and desist immediately, or you
will hear from my lawyers. (They also work for Microsoft. :-)
--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>
Fortunately, I don't recall ever seeing you do that here. Re-posting
an entire article to a newsgroup for your own minor convenience would
be anti-social. (E-mailing it directly to yourself would, of course,
be just fine.)
Actually he is setting the struct member to point to the malloc'd
space.
> > free(ptr->nms);
>
> And here you're trying to free memory that was not allocated by
> malloc(), calloc() or realloc(). BANG! More undefined behaviour! More
> nasal demons! Remember, the nms member of your structure points at a
> string literal.
So this is OK (ish).