[SSSD] [sssd][patch] SYSDB: sysdb_getnetgr returns ENOENT
Lukas Slebodnik
lslebodn at redhat.com
Fri Aug 29 08:14:17 UTC 2014
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 at lists.fedorahosted.org
>>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
>From 517a682750eb0358b76486ede763af4a91963347 Mon Sep 17 00:00:00 2001
>From: Pavel Reichl <preichl at 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
More information about the sssd-devel
mailing list