On (11/07/14 17:28), Pavel Reichl wrote:
On Wed, 2014-06-25 at 10:51 -0400, Simo Sorce wrote:
> On Tue, 2014-06-24 at 17:28 +0200, Pavel Reichl wrote:
> > > Error: PW.NON_CONST_PRINTF_FORMAT_STRING:
> > > sssd-1.11.92/src/db/sysdb_ops.c:109: non_const_printf_format_string:
> > format string is not a string literal, potential security
> > vulnerability if user controlled
> > > sssd-1.11.92/src/db/sysdb_ops.c:109: caretline:
> > >
> > > I prefer to avoid compiler warnings. It will be better to use NULL.
> >
> > (lret = ldb_search(ldb, mem_ctx, _result, base, scope, attrs, NULL);)
> >
> > It's a common practice in SSSD anyway.
>
> Wouldn't it be simpler to use a macro in this case ?
> Your new fuinction hjas to go through variadic expansion prematurely,
> and adds a lot of code:
>
> something like:
> #define sss_ldbn_seartch(ret, arguments, etc., format, ...) {
> do {
> int result = ldb_search(argumets, etc, format, ##__VA_ARGS__);
> ret = result;
> if (res->count == 0) {
> ret = ENOENT;
> }
> } while(0);
>
>
Hello,
sorry for late reply. I guess your approach will definitely save some
lines of code and some CPU cycles.
I just must get used to the idea that macros are not considered evil in
out environment. :-)
Please see attached patch.
Regards,
Pavel Reichl
From d1293b63adcab4298228af225f3af5803759d34b Mon Sep 17 00:00:00
2001
From: Pavel Reichl <preichl(a)redhat.com>
Date: Fri, 11 Jul 2014 15:21:59 +0100
Subject: [PATCH] SYSDB: SSS_LDB_SEARCH - macro around ldb_search
This patch amends previous patch 5153e8b9793dea1e212ca08af0f77ea1d023cbb7.
Macro SSS_LDB_SEARCH is used instead of using fuction sss_ldb_search as
a wrapper around ldb_search which could lead to premature expansion of
variadic parameters.
Part of solution for:
https://fedorahosted.org/sssd/ticket/1991
---
src/db/sysdb.h | 19 +++++++++++++-----
src/db/sysdb_ops.c | 51 -------------------------------------------------
src/db/sysdb_search.c | 10 +++++-----
src/tests/sysdb-tests.c | 38 ++++++++++++++++++------------------
4 files changed, 38 insertions(+), 80 deletions(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 17cd5110c9bdafd8d7b18e621188523ed41e5c8a..a613c84ffa9e4ab6ebdc930a8c4876e69aa3cd13
100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -481,11 +481,6 @@ int sysdb_get_netgroup_attr(TALLOC_CTX *mem_ctx,
const char **attributes,
struct ldb_result **res);
-errno_t sss_ldb_search(struct ldb_context *ldb, TALLOC_CTX *mem_ctx,
- struct ldb_result **_result, struct ldb_dn *base,
- enum ldb_scope scope, const char * const *attrs,
- const char *exp_fmt, ...) SSS_ATTRIBUTE_PRINTF(7, 8);
-
/* functions that modify the databse
* they have to be called within a transaction
* See sysdb_transaction_send()/_recv() */
@@ -864,4 +859,18 @@ errno_t sysdb_search_object_by_sid(TALLOC_CTX *mem_ctx,
const char *sid_str,
const char **attrs,
struct ldb_result **msg);
+
+#define SSS_LDB_SEARCH(ret, ldb, mem_ctx, _result, base, scope, attrs, \
+ exp_fmt, ...) \
+{ \
+ int lret; \
+ \
+ lret = ldb_search(ldb, mem_ctx, _result, base, scope, attrs, exp_fmt, \
+ ##__VA_ARGS__); \
+ ret = sysdb_error_to_errno(lret); \
+ if (ret == EOK && (*_result)->count == 0) { \
+ ret = ENOENT; \
+ } \
+} while(0)
Use do { } while(0); for macro, not just { } while(0)
@see macros SYSDB_VERSION_LOWER_ERROR, SYSDB_VERSION_HIGHER_ERROR,
DEBUG_PAM_DATA ...
Otherwise patch LGTM. If there are no other objections to this version I will
give ACK.
LS