On Wed, Apr 20, 2016 at 02:58:36PM -0400, Simo Sorce wrote: > On Wed, 2016-04-20 at 19:58 +0200, Lukas Slebodnik wrote: > > On (20/04/16 17:21), Jakub Hrozek wrote: > > >On Wed, Apr 20, 2016 at 09:59:19AM -0400, Simo Sorce wrote: > > >> On Wed, 2016-04-20 at 14:16 +0200, Jakub Hrozek wrote: > > >> > On Tue, Apr 05, 2016 at 02:54:10PM -0400, Simo Sorce wrote: > > >> > > On Tue, 2016-04-05 at 12:57 -0400, Simo Sorce wrote: > > >> > > > Thanks, IIRC the int-instead of enum use is intentional, I will > > >> > > > look > > >> > > > at the others. > > >> > > > > >> > > The last coverity/clang thing is a false positive, but I initialized > > >> > > reply to NULL anyway, I expect now it will start complaining of > > >> > > possible > > >> > > NULL dereference :-) > > >> > > > > >> > > Attached find patches that fixes all other issues (hopefully), one of > > >> > > them simply dropped an entire function as it turned out I wasn't > > >> > > using > > >> > > it. > > >> > > > > >> > > Simo. > > >> > > > > >> > > -- > > >> > > Simo Sorce * Red Hat, Inc * New York > > >> > > > >> > > From 4610b546cb37a150ebaee12559c19a17e422708c Mon Sep 17 00:00:00 > > >> > > 2001 > > >> > > From: Simo Sorce <s...@redhat.com> > > >> > > Date: Thu, 7 Jan 2016 10:17:38 -0500 > > >> > > Subject: [PATCH 05/15] Responders: Add support for socket activation > > >> > > > >> > ACK (visual at this point) with a question - do we want to check > > >> > that the fd we received is a UNIX socket using sd_is_socket_unix()? > > >> > > > >> > The sd_listen_fds() manpage recommends that. > > >> > > >> If they recommend it we should, yes. > > > > > >OK, same as with the responder issue, I will prepare a fixup patch and > > >ask you to check it before squashing into your patches.. > > > > > >> > > >> > > From 3755b157de1309f554a380e58c42c38dcd9cc5aa Mon Sep 17 00:00:00 > > >> > > 2001 > > >> > > From: Simo Sorce <s...@redhat.com> > > >> > > Date: Wed, 20 Jan 2016 10:33:39 -0500 > > >> > > Subject: [PATCH 06/15] ConfDB: Add helper function to get > > >> > > "subsections" > > >> > > > > >> > > The secrets database will have "subsections", ie sections that are > > >> > > in the > > >> > > "secrets" namespace and look like this: [secrets/<path>] > > >> > > > > >> > > This function allows to source any section under secrets/ or under > > >> > > any > > >> > > arbitrary sub-path. > > >> > > > > >> > > Related: > > >> > > https://fedorahosted.org/sssd/ticket/2913 > > > > > >[...] > > > > > >> > > +int confdb_get_sub_sections(TALLOC_CTX *mem_ctx, > > >> > > + struct confdb_ctx *cdb, > > >> > > + const char *section, > > >> > > + char ***sections, > > >> > > + int *num_sections) > > >> > > +{ > > >> > > + TALLOC_CTX *tmp_ctx = NULL; > > >> > > + char *secdn; > > >> > > + struct ldb_dn *base = NULL; > > >> > > + struct ldb_result *res = NULL; > > >> > > + static const char *attrs[] = {"cn", NULL}; > > >> > > + char **names; > > >> > > + int base_comp_num; > > >> > > + int num; > > >> > > + int i; > > >> > > > >> > Can you use size_t here so that clang doesn't complain about > > >> > "comparison > > >> > of integers of different signs: 'int' and 'unsigned int'" in the for > > >> > loop below? > > >> > > >> meh, ok :-) > > > > > >Trivial, I can also fix this locally and ask you if it's OK to squash. > > > > > >> > > >> > > + int ret; > > >> > > + > > >> > > + tmp_ctx = talloc_new(mem_ctx); > > >> > > + if (tmp_ctx == NULL) { > > >> > > + return ENOMEM; > > >> > > + } > > >> > > + > > >> > > + ret = parse_section(tmp_ctx, section, &secdn, NULL); > > >> > > + if (ret != EOK) { > > >> > > + goto done; > > >> > > + } > > >> > > + > > >> > > + base = ldb_dn_new(tmp_ctx, cdb->ldb, secdn); > > >> > > + if (base == NULL) { > > >> > > + ret = ENOMEM; > > >> > > + goto done; > > >> > > + } > > >> > > + > > >> > > + base_comp_num = ldb_dn_get_comp_num(base); > > >> > > + > > >> > > + ret = ldb_search(cdb->ldb, tmp_ctx, &res, base, > > >> > > LDB_SCOPE_SUBTREE, > > >> > > + attrs, NULL); > > >> > > + if (ret != LDB_SUCCESS) { > > >> > > + ret = EIO; > > >> > > + goto done; > > >> > > + } > > >> > > + > > >> > > + names = talloc_zero_array(tmp_ctx, char *, res->count + 1); > > >> > > + if (names == NULL) { > > >> > > + ret = ENOMEM; > > >> > > + goto done; > > >> > > + } > > >> > > + > > >> > > + for (num = 0, i = 0; i < res->count; i++) { > > >> > > + const struct ldb_val *val; > > >> > > + char *name; > > >> > > + int n; > > >> > > + int j; > > >> > > > >> > Every time I see variables declared in a scope in C except loop control > > >> > variables I think "This should be a static function of its own" :-) > > >> > > >> Should it be in this case ? </lazy> > > > > > >Not sure, I'll make up my mind when I fix the other trivial issues. > > > > > > > > >[...] > > > > > >> > > From aa6203a0a6cb1f3ac60428887e77fe176489c3e0 Mon Sep 17 00:00:00 > > >> > > 2001 > > >> > > From: Christian Heimes <chei...@redhat.com> > > >> > > Date: Fri, 8 Jan 2016 13:26:22 +0100 > > >> > > Subject: [PATCH 08/15] Secrets: m4 macros for jansson and http-parser > > >> > > > > >> > > Prepares autoconf for the new Secrets Provider dependencies > > >> > > > > >> > > Related: > > >> > > https://fedorahosted.org/sssd/ticket/2913 > > >> > > > > >> > > > >> > [...] > > >> > > > >> > > +PKG_CHECK_MODULES([HTTP_PARSER], [http_parser], > > >> > > [found_http_parser=yes], [found_http_parser=no]) > > >> > > > >> > There is no pkgconfig for http-parser-devel, so it seems to be this > > >> > line > > >> > is redundant. > > >> > > > >> > Otherwise ACK. > > >> > > >> I wonder why this is not failing then ? > > > > > >Because we find the library using the next AC_CHECK_LIB call. Again, I > > >will fixup this locally and send a branch for review later. > > > > We have PKG_CHECK_MODULES and then fallback to AC_CHECK_LIB AC_CHECK_HEADER > > on many places. It's not a problem from my POV. > > > > If we would like to simplify it. We could get rid of fallback detection on > > many > > places. And http_parser can provide pkgconfig file in future. > > > > I would prefer to be "kind of consistent" in detection. > > But I do not insist on it. > > I am not sure if you arguing to keep the checks as they are in my patch > or to remove the pkgconfig based check :) > > Simo.
Given that Lukas says "http_parser can provide pkgconfig in future", I read his mail as a preference to keep the pkg-check test. And actually I agree, it doesn't hurt, let's keep it in. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
