[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