On 07/24/2014 05:53 PM, Michal Židek wrote:
On 07/24/2014 11:28 AM, Lukas Slebodnik wrote:
ehlo,
This patch add possibility to replace whitespace in user and group
names with
a specified string. With string "-", sssd will return the same result as
winbind enabled option "winbind normalize names"
Resolves:
https://fedorahosted.org/sssd/ticket/1854
patch is attached.
LS
Hi Lukas,
src/config/SSSDConfig/__init__.py.in
76 'whitespaces_replacement_string': _('All white spaces will be
replaced in group name with this string'),
Not just group name, but also user name. IMO something
like this would be better:
"All white spaces in group or user names will be replaced with
this string." Or something similar.
Also please put changes in
src/config/SSSDConfig/__init__.py.in and man page changes into a
separate patch. We will than ping a native speaker (Stephen ?)
to check the wording when the coding part is accepted.
The option name could be at least a little shorter, maybe:
replace_whitespace_str
or
replace_spaces_by
But this is not a strong requirement, if you do not like the above
names and can't come up with something better, we can stick to the
current name.
35 static const char *replace_whitespaces(TALLOC_CTX *mem_ctx,
... <snip>
76 while (*ptr != '\0') {
77 if (isspace(*ptr)){
78 new_name = talloc_asprintf_append(...
79 } else{
^^^^^
Missing space after else
80 new_name = talloc_asprintf_append(...
81 }
The patch seem to work fine. But functions like
replace_whitespaces and the reverse version can be
easily unit-tested. It would be great if you could
add unit tests for these two.
Michal
Also when you call the new functions, you use this pattern
887 name = reverse_replace_withespaces(dctx, name,
888
889 if (name == NULL) {
890 DEBUG(SSSDBG_CRIT_FAILURE,
891 "reverse_replace_withespaces...
892 continue;
893 }
Other functions in this part set ret and jump to done
if something goes wrong. We could do the same here.
Michal
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
