[SSSD] [PATCH] NSS: replace with space with specified character in names.
Dan Lavu
side_control at runlevelone.net
Thu Jul 24 19:20:45 UTC 2014
Was talking to Lukas about the name and came up with this suggestion, so instead of "whitespaces_replacement_string" to "override_default_whitespace", thoughts?
Dan
----- Original Message -----
From: "Michal Židek" <mzidek at redhat.com>
To: "Development of the System Security Services Daemon" <sssd-devel at lists.fedorahosted.org>
Sent: Thursday, July 24, 2014 12:12:49 PM
Subject: Re: [SSSD] [PATCH] NSS: replace with space with specified character in names.
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
sssd-devel at lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140724/5e63def7/attachment.html>
More information about the sssd-devel
mailing list