[SSSD] [PATCH] NSS: replace with space with specified character in names.

Michal Židek mzidek at redhat.com
Fri Jul 25 13:15:07 UTC 2014


On 07/25/2014 01:42 PM, Lukas Slebodnik wrote:
> On (24/07/14 17:53), 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.
>>
>
> Attached patches fix also comments from other mails in this thread.
>
> LS
>

Thanks for adding the unit tests!

The patches look good to me and passed my testing. So ACK for
the code part. I would like to have a native English speaker
to review the man page changes in src/man/sssd.conf.5.xml

Michal



More information about the sssd-devel mailing list