[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