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.