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(a)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