On (13/12/13 10:35), Jakub Hrozek wrote: >On Thu, Dec 12, 2013 at 09:54:51PM +0100, Jakub Hrozek wrote: >> On Thu, Dec 12, 2013 at 01:20:59PM +0100, Pavel Březina wrote: >> > >I see a number of compilation warnings with the patches (using clang-3.3) >> > > >> > >/home/remote/jhrozek/devel/sssd/src/providers/ldap/sdap_async_initgroups_ad.c:785:9: >> > > warning: variable 'in_transaction' is used uninitialized whenever 'if' >> > >condition is true [-Wsometimes-uninitialized] >> > > if (ret != EOK) { >> > > ^~~~~~~~~~ >> > >/home/remote/jhrozek/devel/sssd/src/providers/ldap/sdap_async_initgroups_ad.c:873:9: >> > > note: uninitialized use occurs here >> > > if (in_transaction) { >> > > ^~~~~~~~~~~~~~ >> > >/home/remote/jhrozek/devel/sssd/src/providers/ldap/sdap_async_initgroups_ad.c:785:5: >> > > note: remove the 'if' if its condition is always false >> > > if (ret != EOK) { >> > > ^~~~~~~~~~~~~~~~~ >> > >/home/remote/jhrozek/devel/sssd/src/providers/ldap/sdap_async_initgroups_ad.c:778:9: >> > > warning: variable 'in_transaction' is used uninitialized whenever 'if' >> > >condition is true [-Wsometimes-uninitialized] >> > > if (groups == NULL) { >> > > ^~~~~~~~~~~~~~ >> > >> > I also seen following warning with gcc and higher optimization level: >> > /home/pbrezina/workspace/sssd/src/providers/ldap/sdap_async_initgroups_ad.c: >> > In function 'sdap_ad_tokengroups_initgr_posix_tg_done': >> > /home/pbrezina/workspace/sssd/src/providers/ldap/sdap_async_initgroups_ad.c:1008:20: >> > warning: 'num_sids' may be used uninitialized in this function >> > [-Wmaybe-uninitialized] >> > /home/pbrezina/workspace/sssd/src/providers/ldap/sdap_async_initgroups_ad.c: >> > In function 'sdap_ad_tokengroups_initgr_mapping_done': >> > /home/pbrezina/workspace/sssd/src/providers/ldap/sdap_async_initgroups_ad.c:873:8: >> > warning: 'in_transaction' may be used uninitialized in this function >> > [-Wuninitialized] >> > /home/pbrezina/workspace/sssd/src/providers/ldap/sdap_async_initgroups_ad.c:777:14: >> > warning: 'num_sids' may be used uninitialized in this function >> > [-Wmaybe-uninitialized] >> > >> > New patches attached. Here's the diff. >> > >> > --- a/src/providers/ldap/sdap_async_initgroups_ad.c >> > +++ b/src/providers/ldap/sdap_async_initgroups_ad.c >> > @@ -746,14 +746,14 @@ static void >> > sdap_ad_tokengroups_initgr_mapping_done(struct tevent_req *subreq) >> > const char *name = NULL; >> > const char *sid = NULL; >> > char **sids = NULL; >> > - size_t num_sids; >> > + size_t num_sids = 0; >> > size_t i; >> > time_t now; >> > gid_t gid; >> > char **groups = NULL; >> > size_t num_groups; >> > errno_t ret, sret; >> > - bool in_transaction; >> > + bool in_transaction = false; >> > >> > tmp_ctx = talloc_new(NULL); >> > if (tmp_ctx == NULL) { >> > @@ -977,7 +977,7 @@ sdap_ad_tokengroups_initgr_posix_tg_done(struct >> > tevent_req *subreq) >> > const char *name = NULL; >> > char *sid = NULL; >> > char **sids = NULL; >> > - size_t num_sids; >> > + size_t num_sids = 0; >> > char **valid_groups = NULL; >> > size_t num_valid_groups; >> > char **missing_sids = NULL; >> > >> > >> >> Thank you, I think these patches are fine so ACK from me. But I would >> like to run them through Coverity, so I'll push them when the scan >> finishes. >> >> Thanks for the big patches! > >Hi Pavel, >sorry but there are some Coverity warnings with the latest iteration of >patches. The short version is: > >Error: UNINIT (CWE-457): [#def1] >sssd-1.11.90/src/providers/ldap/sdap_async_initgroups_ad.c:756: >var_decl: Declaring variable "in_transaction" without initializer. >sssd-1.11.90/src/providers/ldap/sdap_async_initgroups_ad.c:873: >uninit_use: Using uninitialized value "in_transaction". > >Error: COMPILER_WARNING: [#def2] >sssd-1.11.90/src/providers/ldap/sdap_async_initgroups_ad.c: scope_hint: >In function 'sdap_ad_tokengroups_initgr_mapping_done' >sssd-1.11.90/src/providers/ldap/sdap_async_initgroups_ad.c:873:8: >warning: 'in_transaction' may be used uninitialized in this function >[-Wmaybe-uninitialized] ># if (in_transaction) { ># ^ > >Error: FORWARD_NULL (CWE-476): [#def3] >sssd-1.11.90/src/providers/ldap/sdap_async_initgroups_ad.c:741: >assign_zero: Assigning: "state" = "NULL". >sssd-1.11.90/src/providers/ldap/sdap_async_initgroups_ad.c:874: >var_deref_op: Dereferencing null pointer "state". > >I'll send you the e-mail with the full report (sorry, can't paste it >here, the Coverity instance is behind RH firewall).
It is very likely that you applied old version of Pavel's patches. I didn't found any warnings from static analysers with the latest patches. ACK++ LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel