[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