On (28/08/14 18:44), Pavel Reichl wrote:
On 08/28/2014 06:19 PM, Jakub Hrozek wrote:
>On Thu, Aug 28, 2014 at 11:49:38AM +0200, Pavel Reichl wrote:
>>>>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
>>>>
>I have one small comment, sorry for coming late into the thread...
>
>
>>-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() */
>>@@ -901,4 +896,16 @@ errno_t sysdb_gpo_get_gpos(TALLOC_CTX *mem_ctx,
>> struct sss_domain_info *domain,
>> struct ldb_result **_result);
>>+#define SSS_LDB_SEARCH(ret, ldb, mem_ctx, _result, base, scope, attrs, \
>>+ exp_fmt, ...) do { \
>>+ int lret; \
>Can you rename the variable "lret" here? Calling it "lret" might
clash with
>some other variable from outer scope..
>
>Maybe "_sls_lret" ? (_sss_ldb_search_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)
>>+
Thanks, for noticing. New patch attached.
>_______________________________________________
>sssd-devel mailing list
>sssd-devel(a)lists.fedorahosted.org
>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
From 517a682750eb0358b76486ede763af4a91963347 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 | 17 ++++++++++++-----
src/db/sysdb_ops.c | 51 -------------------------------------------------
src/db/sysdb_search.c | 10 +++++-----
src/tests/sysdb-tests.c | 38 ++++++++++++++++++------------------
4 files changed, 36 insertions(+), 80 deletions(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 3cef1e66bdc5c3870ed1a349c797e66ac4ad3475..2fc8ed529d51a58e949bcb788bba56e8bd7d7e11
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() */
@@ -901,4 +896,16 @@ errno_t sysdb_gpo_get_gpos(TALLOC_CTX *mem_ctx,
struct sss_domain_info *domain,
struct ldb_result **_result);
+#define SSS_LDB_SEARCH(ret, ldb, mem_ctx, _result, base, scope, attrs, \
+ exp_fmt, ...) do { \
+ int _sls_lret; \
+ \
+ _sls_lret = ldb_search(ldb, mem_ctx, _result, base, scope, attrs, \
+ exp_fmt, ##__VA_ARGS__); \
+ ret = sysdb_error_to_errno(_sls_lret); \
+ if (ret == EOK && (*_result)->count == 0) { \
+ ret = ENOENT; \
+ } \
+} while(0)
+
#endif /* __SYS_DB_H__ */
You moved new macro to the end of file, but there is "GPO section".
@see comment in sysdb.h "/* === Functions related to GPOs === */"
More suitable would be section "/* Search Entry */"
LS