On 6/8/07, Jan Mura <jan...@vo...> wrote:
>
> Hello,
>
> I would like to ask about some things in linked list I
> try to write.
> It looks like:
>
> struct s
> {
> int id;
> char *name;
> struct s *next;
> };
>
> int main ()
> {
> struct s *front = NULL; /*the front of a list */
> int i = 1;
> char *name;
>
> while (1)
> {
> gets (name);
> if (strlen(name) == 0) break;
> in_list (&front, i, name);
> i++;
> }
> return 0;
> }
>
> void in_list (struct s **q, int in_id, const char *in_name)
> {
> struct s*member, *temp;
>
> member = malloc (sizeof (struct s));
> member -> name = malloc (strlen (in_name) +1);
>
> member-> id = in_id;
> strcpy (member -> name, in_name);
> member -> next = NULL; /* it will be the last member in the list */
>
> if (q == NULL) /* empty list */
> {
> q = member;
> q -> next = NULL;
> }
> else
> {
> temp = q;
> temp -> next = q -> next;
>
> while (temp -> next != NULL)
> temp = temp -> next; /* looking for the last member in the list */
>
> temp ->next = member; /* the last member is no more the last one */
> }
> }
>
> Should I allocate the space for temp and temp -> next when I am
> putting an adress from member to temp?
>
> So should it looks like:
>
> temp = malloc ...;
> temp = member;
> ???
>
> Thanks for your answer.
>
> Jan Mura
> jan...@vo...
>
> Should I allocate the space for temp and temp -> next when I am
> putting an adress from member to temp?
No, you shouldn't: /temp/ is a local variable that points to something, when
you declare /temp/ thus : /struct * temp;/ you are already getting local
space to store the pointer, just as if you were declaring an integer
(pointers _are_ integers, actually)
Your suggestion would allocate some memory, and then immediately lose the
pointer to it, losing the memory without even using it.
Also, there are a couple of small errors in your code that show:
/struct **q/ points to the pointer to the first member, therefore you should
use / *q/ instead of /q/ to access the pointer to the first member
(basically all your uses of /q/)
When you say / *q->next = NULL;/, *q is == member, so / *q->next/ is already
NULL
Same thing here: /temp -> next = *q -> next;/ :temp and *q point to the same
structure, so there's no use doing that
Plus, just some hints, if I may:
you can avoid using /struct s/ everywhere like this:
typedef struct list_element_struct {
int id;
char *name;
struct list_element_struct *next; //you still need struct here becuse the
typedef isn't done yet
} list_element;
and then use /list_element/ instead
you might want to write some helper functions such as these:
list_element *new_list_element(int id,char*name) // mallocs the element and
sets the name
void free_list_element(list_element *elem) //frees the name and element
void list_append(list_element **list, list_element *elem) // does the
appending part (the rest of your in_list)
Of course, you can do whatever you want :-D
Hope it helps :)
Jonathan
--
<Morpheus> linux, c'est une question de VI ou de MORE
|
