On Tue, Feb 12, 2013 at 11:44:25AM +0100, Pavel Březina wrote:
On 02/01/2013 12:32 PM, Pavel Březina wrote:
>On 01/31/2013 08:07 PM, Jakub Hrozek wrote:
>>On Thu, Jan 31, 2013 at 11:52:53AM +0100, Pavel Březina wrote:
>>>https://fedorahosted.org/sssd/ticket/1795
>>
>>> From aba6be77012e2c4f741d78e887f9524b869b78ae Mon Sep 17 00:00:00 2001
>>>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
>>>Date: Thu, 31 Jan 2013 11:21:06 +0100
>>>Subject: [PATCH 1/2] util: add replace_char() function
>>>
>>>Replace all occurrences of character in string with other character.
>>
>>Please add a unit test for new utility functions. I know that this is
>>a trivial C function, but in general I think we should start to be far
>>stricter especially with new code now that we are not rushing for a
>>deadline.
>
>No, thank you.
>
>
>Just kidding :-) I switched to the condition below, which is not generic
>enough to create a separate utility function.
>
>>
>>I would also say that we should be even more defensive because currently
>>the libkrb5 code only allows these file names (see valid_name() function
>>in src/util/profile/prof_parse.c):
>>
>>if (!isalnum((unsigned char)*p) && *p != '-' && *p !=
'_')
>> return invalid;
>>
>>So in my opinion we should replace any character that doesn't match the
>>above. Dot would be the typical case, but I don't think we guarantee
>>that other weird characters don't appear in the domain name. Debugging
>>why the file didn't load would then be very hard.
>
>I tried $ and @ characters in addition to dot and we fail to initialize
>D-Bus with those characters present. They must be escaped to be used in
>D-Bus.
>
>>Maybe make it clear with the function name that the replace is not done
>>in-place?
>>
>>>+{
>>>+ char *str = NULL;
>>>+ char *pos = NULL;
>>>+
>>
>>You don't need to initialize the pointers here as they are assigned on
>>the next line..
>
>I know. It is my habit to always initialize variables (except ret).
>
>New patch is attached.
Rebased patch is attached.
This patch looks good to me, Ack.
I also filed
https://fedorahosted.org/sssd/ticket/1809 to track a safer
method.