On 25/07/12 11:01, Pavel Březina wrote:
Nack.
Things to consider:
- support reading password directly from command line
- -p 123 | -password 123
- -f file | --password-file file
- support reading everything from file that contains users in passwd
format
Please include the manual page in src/man/Makefile.am and src/man
/po/po4a.cfg so it can be built and translated.
A forward backslash is used to close a tag (ie </para> instead of
<\para>.
sss_seed.c:
Mark the functions as static.
Don't use tools_ctx and octx and create a custom structure (say
seed_ctx) that will fit exactly to your needs.
Our convention is to use tmp_ctx for temporary contexts.
You don't use enum seed_input_type anywhere.
180: Missing NULL check.
191: Typo "Passowrds"
197: Create a local variable "char *password" and function parameter
"char **_password". Work with the "password" and then steal it at the
end:
*_password = talloc_steal(mem_ctx, password).
seed_interactive_input(): Debug messages would be nice here when
something fails.
296: This check should be done at the beginning of the function.
300: Use lower case in "domain".
319, 320: The same thing as with the password variable.
main(): Load every option into separate variable (pc_*) and
talloc_strdup() that into the structure (see sss_usermod) so you don't
mix malloc and talloc pointers.
490: Use CHECK_ROOT macro here.
500: You should use fully qualified name (user@domain) to be sure that
you get the correct user in multi domain environment. If UID is
specified you should test if it matches passwd->pw_uid.
Use errno to get proper error code from getpwnam(). The good way would
be:
errno = 0;
pc_passwd = getpwnam(...);
if (pc_passwd == NULL) {
ret = errno;
DEBUG(..., ("... [%d]: %s\n", ret, strerror(ret)));
goto done;
}
You should copy (talloc_strdup()) passwd values you need instead of
simple assignment because it points to a static location which may be
overwritten by other calls.
Also name pc_passwd doesn't make sense as it has nothing in common
with popt context.
518: You're missing space after comma in the comment.
Put everything after line 579 into a separate function.
Thanks,
Pavel.
Thanks, Pavel. I covered most of what you noted. I forgot to include one
commit with the last patch I sent, which left the interactive input
broken in the last patch. That should be fixed along with a shorter,
more readable main function.