On (01/09/14 11:20), Pavel Reichl wrote:
On 08/29/2014 10:14 AM, Lukas Slebodnik wrote:
>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.
>>
>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 */"
OK, makes sense. Please see attached patch.
>
From 0eb33210da528cedfb423db2107e87a9ef61cccc 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
---
ACK
LS