[systemd-bugs] [Bug 82004] RFE: The tty-ask-password-agent does not use all devices of /dev/console

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=82004

--- Comment #5 from Werner Fink <werner at suse.de> ---
(In reply to Zbigniew Jedrzejewski-Szmek from comment #1)

Found some time to work on that

> Looks reasonable.
> 
> I have some requests for changes in the implementation though:
> 
> 1. Please make the list of 'consoles' simply an array. Then GREEDY_REALLOC
> will be enough in a loop will be enough to initially allocate and add items
> to the array, making everything a bit simpler.

Done ... even if I prever double linked lists.

> 
> Also, having the console name appended to the end of 'struct console' makes
> things more complicated than need be. Please just strndup() the string.

I'd like to be able to free the structure in whole, with the attached patch
this should work as I've tested.

> 2. 'consoles' global variable does not have to be global, and 'current_dev'
> probably too. We usually only use global variables for commandline arguments.

Now 'consoles' is local.  Nevertheless 'current_dev' remains global to be able
to use this parse_password()

> 3. The whole thing with 'usemask' is not going to work, because there's no
> locking. And I don't see the need for it. Isn't it enough to
>     a) in a loop: set index, launch child
>     b) wait until no children are left
>     c) quit
> ?

Hmmm ... the 'usemask' is a shared memory area used to allow the worker childs
asking for passwords to communicate with the main process. For this the id is
used as bit. This allows to continue if one of the consoles had been used for a
password as afterwards the remaining asking processes will be terminated.  The
wait_for_terminate() does not help for this use case.

> 4. In collect consoles the block which adds a new 'struct console' entry is
> repeated twice. Can you make this a separate function?

done

> 5. For consistency with other code: switch(fork) should be turned into an
> two if's (e.g.
> http://cgit.freedesktop.org/systemd/systemd/tree/src/core/execute.c#n797)

done

> 6. When log_oom() happens, just fail, do not continue.

done

> 
> 7. Killing and waiting for child should (I think) use the same signal
> sequence as wait_for_terminate(). If something different is needed, please
> add a comment.

see above for the description of the usage of 'usemask' ... if one process has
handled a password the other asking workers should die, that is we do not want
to wait for termination.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/systemd-bugs/attachments/20150703/306b7039/attachment.html>