<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW - RFE: The tty-ask-password-agent does not use all devices of /dev/console" href="https://bugs.freedesktop.org/show_bug.cgi?id=82004#c5">Comment # 5</a> on <a class="bz_bug_link bz_status_NEW " title="NEW - RFE: The tty-ask-password-agent does not use all devices of /dev/console" href="https://bugs.freedesktop.org/show_bug.cgi?id=82004">bug 82004</a> from <a class="email" href="mailto:werner@suse.de" title="Werner Fink <werner@suse.de>"> Werner Fink</a> <pre>(In reply to Zbigniew Jedrzejewski-Szmek from <a href="show_bug.cgi?id=82004#c1">comment #1</a>) 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. > <a href="http://cgit.freedesktop.org/systemd/systemd/tree/src/core/execute.c#n797">http://cgit.freedesktop.org/systemd/systemd/tree/src/core/execute.c#n797</a>) 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.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the QA Contact for the bug.</li> <li>You are the assignee for the bug.</li> </ul> </body> </html>