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