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.