On 07/17/2014 06:12 PM, Pavel Reichl wrote:
On Wed, 2014-07-16 at 15:40 +0200, Michal Židek wrote:
Hi,
patches for ticket https://fedorahosted.org/sssd/ticket/2367 are in attachment.
Michal
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hi Michal,
I haven't tested the patches yet. I just wanted to share some nitpicks with you:
From 071b3ca8cd9a194c8cb287f9abca2fe7c58323a2 Mon Sep 17 00:00:00 2001 From: Michal Zidek mzidek@redhat.com Date: Tue, 15 Jul 2014 12:00:36 -0400 Subject: [PATCH 1/3] Add function confdb_set_string.
src/confdb/confdb.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/confdb/confdb.h | 6 +++++ 2 files changed, 76 insertions(+)
diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index 15de961..79c89b7 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -369,6 +369,76 @@ done: return ret; }
+int confdb_set_string(struct confdb_ctx *cdb,
const char *section,const char *attribute,char *val)+{
- TALLOC_CTX *tmp_ctx;
- struct ldb_dn *dn;
- char *secdn;
- struct ldb_message *msg;
- int ret, lret;
- tmp_ctx = talloc_new(NULL);
- if (!tmp_ctx)
return ENOMEM;Although there's not a consensus about the form of checking allocated pointers ( !tmp_ctx vs. tmp_ctx != NULL ) among SSSD developers, still I think there is an agreement that 'if' should be followed by block or condition and action should be one-liner.
Could you change the code to something like?
if (!tmp_ctx) return ENOMEM;
or
if (!tmp_ctx) { return ENOMEM; }
Fixed.
In the second patch you use 2 forms of testing result of strcasecmp
!strcasecmp(tmp, "true") strcasecmp(domain->provider, "local") == 0
Could you please just use one of them? From my POV the second is preferred.
Fixed.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thank you Pavel! New version is attached.
Michal