On Mon, Oct 01, 2012 at 07:45:17PM +0200, Pavel Březina wrote:
On 10/01/2012 06:25 PM, Michal Židek wrote:
>Patch 01: The debug_level variable was set before the parameters were
>processed, so the default debug level was used no matter what number was
>specified with --debug.
>
>Patch 02: When interactive input fails, error message should be displayed.
>https://fedorahosted.org/sssd/ticket/1549
>
>Patch 03: When file is used to specify a password in sss_seed, then only
>first line of this file is used. Also empty passwords are treated as
>errors. (My local test environment is broken so I have not tested this,
>please test it when reviewing.)
>https://fedorahosted.org/sssd/ticket/1548
>
>Patches are in attachment.
>
>Michal
Patch 1: nack
Instead of moving debug level initialization further in the code,
you should parse the parameters earlier. The other tools uses
following pattern:
- set debug program name
- set locale
- parse parameters
- set debug level
See sss_groupadd for example.
Patch 2: ack
Patch 3: I'll do this one tomorrow.
My take on Patch 3:
Instead of stripping everything after the first line, I would prefer
simply failing if there's any non-whitespace character after the first
line break ('\n').
Stripping anything after the first line unconditionally may seem like
everything was OK to the user..while he wouldn't be able to easily input
a password like that.
Also a code-style nitpick -- please put a newline after "}" and before
the comment saying "/* Do not allow empty passwords */" in
seed_password_input_prompt().