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

Lukas Slebodnik lslebodn at redhat.com
Tue May 13 15:51:52 UTC 2014


On (12/05/14 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?
>> > 
>> > Hi, sure.
>> > 
>> > You're leaking memory in ifp_bus_path_escape/ifp_bus_path_unescape when 
>> > the append operation fails, but I think we can keep it that way since 
>> > the mem_ctx is bounded to a request which is supposed to end shortly.
>> > 
>> > Cleaner solution would be to free the path on error, but I'll keep it up 
>> > to yours decision.
>> > 
>> > Ack from my side.
>> 
>> I think it would be nicer to to free the buffer on error, that would
>> also allow us to use leak checks in the unit tests.
>> 
>> I'll send new versions.
>
>Thanks for the review, new patches are attached.

>From 428c7e9febf470ca737ab93e2cce3b20ee4f482b Mon Sep 17 00:00:00 2001
>From d06ba377f0cba19982dc3ce1d192cdb4919662a0 Mon Sep 17 00:00:00 2001
>From: Pavel Březina <pbrezina at redhat.com>
>Date: Sun, 11 May 2014 16:18:06 +0200
>Subject: [PATCH 2/3] IFP: Add a utility function to reply with an object path
>
>---
> src/responder/ifp/ifp_private.h |  6 ++++++
> src/responder/ifp/ifpsrv_util.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 47 insertions(+)
>
>diff --git a/src/responder/ifp/ifp_private.h b/src/responder/ifp/ifp_private.h
>index 286cc75c9b774b0fd9d18ef6166a7c3f1ae41e53..580be9abd8923c834250c7882af0512ae6490003 100644
>--- a/src/responder/ifp/ifp_private.h
>+++ b/src/responder/ifp/ifp_private.h
>@@ -73,6 +73,12 @@ const char *ifp_path_strip_prefix(const char *path, const char *prefix);
> char *ifp_bus_path_unescape(TALLOC_CTX *mem_ctx, const char *path);
> char *ifp_bus_path_escape(TALLOC_CTX *mem_ctx, const char *path);
> 
>+char *_ifp_reply_objpath(TALLOC_CTX *mem_ctx, const char *base,
>+                         const char *part, ...);
>+
>+#define ifp_reply_objpath(mem_ctx, base, ...) \
>+    _ifp_reply_objpath(mem_ctx, base, ##__VA_ARGS__, NULL)
>+
> errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict,
>                                struct ldb_message_element *el);
> const char **ifp_parse_attr_list(TALLOC_CTX *mem_ctx, const char *conf_str);
>diff --git a/src/responder/ifp/ifpsrv_util.c b/src/responder/ifp/ifpsrv_util.c
>index 119e0d277664905a462c104abb6305258af2acca..b7094560fc2f00ee407e7bf78faac82b7c3b91ee 100644
>--- a/src/responder/ifp/ifpsrv_util.c
>+++ b/src/responder/ifp/ifpsrv_util.c
>@@ -203,6 +203,47 @@ char *ifp_bus_path_unescape(TALLOC_CTX *mem_ctx, const char *path)
>     return safe_path;
> }
> 
>+char *
>+_ifp_reply_objpath(TALLOC_CTX *mem_ctx, const char *base,
>+                   const char *part, ...)
>+{
>+    char *safe_part;
>+    char *path = NULL;
>+    va_list va;
>+
>+    if (base == NULL) {
>+        DEBUG(SSSDBG_OP_FAILURE, "Wrong object path base!\n");
>+        return NULL;
>+    }
>+
>+    path = talloc_strdup(mem_ctx, base);
>+    if (path == NULL) return NULL;
>+
>+    va_start(va, part);
      ^^^^^^^^^^^^^^^^^^
      start 
>+    while (part != NULL) {
>+        safe_part = ifp_bus_path_escape(mem_ctx, part);
>+        if (safe_part == NULL) {
>+            DEBUG(SSSDBG_OP_FAILURE, "Could not add [%s] to objpath\n", part);
>+            goto fail;
              ^^^^^^^^^^
             in case of failure va_end is not called.
>+        }
>+
>+        path = talloc_asprintf_append(path, "/%s", safe_part);
>+        talloc_free(safe_part);
>+        if (path == NULL) {
>+            goto fail;
              ^^^^^^^^^^
             in case of failure va_end is not called.
>+        }
>+
>+        part = va_arg(va, const char *);
>+    }
>+    va_end(va);
>+
>+    return path;
>+
>+fail:
>+    talloc_free(path);
>+    return NULL;
>+}
>+
> errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict,
>                                struct ldb_message_element *el)
> {
>-- 
>1.9.0
>

reported by coverity

LS



More information about the sssd-devel mailing list