[SSSD] [PATCH] IFP: Add a utility function to reply with an object path

Lukas Slebodnik lslebodn at redhat.com
Thu May 15 18:54:53 UTC 2014


On (15/05/14 20:32), Jakub Hrozek wrote:
>On Tue, May 13, 2014 at 06:57:28PM +0200, Stef Walter wrote:
>> On 12.05.2014 17:09, Jakub Hrozek wrote:
>> > On Sun, May 11, 2014 at 10:58:24PM +0200, Jakub Hrozek wrote:
>> >> On Sun, 2014-05-11 at 19:18 +0200, Pavel Březina wrote:
>> >>> On 05/11/2014 04:40 PM, Jakub Hrozek wrote:
>> >>>> Hi,
>> >>>>
>> >>>> the attached patches add utility functions that allow the InfoPipe
>> >>>> responder to reply with an object path, escaped if needed.
>> >>>>
>> >>>> The ifp_reply_objpath() function was initially in my tree, but Pavel
>> >>>> improved it quite a bit and fixed some bugs, so the patch should be
>> >>>> attributed to him. I'm also fine with his code...Pavel, can you
>> >>>> ack^Wreview the other two?
>> 
>> This is a really ugly part of DBus :S
>> 
>> I wish object paths weren't so restrictive. There's no real reason for
>> it, IMO. But oh well, this is what we have to live with.
>> 
>> > +        /* D-Bus spec says:
>> > +         * *
>> > +         * * Each element must only contain the ASCII characters
>> > +         * "[A-Z][a-z][0-9]_"
>> > +         * */
>> > +        if ((c >= 'A' && c <= 'Z')
>> > +                || (c >= 'a' && c <= 'z')
>> > +                || (c >= '0' && c <= '9')
>> > +                || c == '_') {
>> > +            safe_path = talloc_asprintf_append_buffer(safe_path, "%c", c);
>> > +            if (safe_path == NULL) {
>> > +                goto done;
>> > +            }
>> 
>> If you're expecting to unescape these paths, then you have to escape the
>> underscore as well, and not let it through verbatim.
>
>Ah, correct, thanks!
>
>> 
>> > +    /* Special case for the empty string */
>> > +    if (strcmp(path, "_") == 0) {
>> 
>> I may be confused, but I don't see similar logic for empty strings in
>> ifp_bus_path_escape().
>
>That's also a bug.
>
>I added unit checks for both these scenarios to the
>test_path_escape_unescape() unit test.
>
>> 
>> Stef
>
>Thanks for the review!
>
>new patches are attached.
>
You forgot to fix coverity issue
https://lists.fedorahosted.org/pipermail/sssd-devel/2014-May/019503.html

LS



More information about the sssd-devel mailing list