Re: [util-linux] [PATCH 00/13] Initial import of sulogin

[Karel Zak <kzak@redhat.com>]

On Fri, Nov 09, 2012 at 09:38:45AM +0100, Karel Zak wrote:
> On Fri, Oct 12, 2012 at 04:07:57PM +0200, Dr. Werner Fink wrote:
> > +static
> > +#ifdef __GNUC__
> > +__attribute__((__nonnull__,__hot__))
> > +#endif
> > +void consalloc(char * name)
> > +{
> > +	struct console *restrict tail;
> > +
> > +	if (posix_memalign((void*)&tail, sizeof(void*), alignof(typeof(struct console))) != 0)
> > +		perror("memory allocation");
> 
>  why not malloc() ?
> 
> > +
> > +	tail->next = (struct console*)0;
> > +	tail->tty = name;
> > +
> > +	tail->file = (FILE*)0;
> > +	tail->flags = 0;
> > +	tail->fd = -1;
> > +	tail->id = concount++;
> > +	tail->pid = 0;
> > +	memset(&tail->tio, 0, sizeof(tail->tio));
> > +	memcpy(&tail->cp, &initcp, sizeof(struct chardata));
> > +
> > +	if (!consoles)
> > +		consoles = tail;
> > +	else
> > +		consoles->next = tail;
> > +}
> 
>  It does not look like a list. It always update the same pointer
>  consoles->next but not "consoles", so the result is list with only
>  two items (first and the last item).
> 
>  If you want to create a list you have to always update two pointers
>  (the "->next" and pointer to the first/last item).

 Note that the right code is something like:

    struct console *last;

    [...]

	for (last = consoles; last && last->next; last = last->next);

	if (!last)
		consoles = tail;
	else
		last->next = tail;

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com