On Mon, Dec 16, 2013 at 10:40:46AM +0100, Pavel Březina wrote:
> On 12/13/2013 10:35 AM, 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).
> >
>
> Hi,
> are you sure you applied the latest patches? Because these issues
> were fixed there.
>
No, I'm not actually because I no longer have the SRPM I tested handy.
But I'm about to put my latest patches through Coverity so I'll run your
as well.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
