ehlo,
Clang static analyzer assume that ldb_search can found 0 entries in the tree "cn=sysdb". Thenvariable version could be used uninitialized.
We cannot get to such state in sssd but we already handle a case for more then one entry.
LS
Lukaš,
On Tue, Aug 30, 2016 at 4:54 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
ehlo,
Clang static analyzer assume that ldb_search can found 0 entries in the tree "cn=sysdb". Thenvariable version could be used uninitialized.
We cannot get to such state in sssd but we already handle a case for more then one entry.
I don't think this is the right approach as res->count == 0 seems to be a valid case for a newly created database (please, correct me if I'm wrong).
Best Regards, -- Fabiano Fidêncio
On (30/08/16 16:59), Fabiano Fidêncio wrote:
Lukaš,
On Tue, Aug 30, 2016 at 4:54 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
ehlo,
Clang static analyzer assume that ldb_search can found 0 entries in the tree "cn=sysdb". Thenvariable version could be used uninitialized.
We cannot get to such state in sssd but we already handle a case for more then one entry.
I don't think this is the right approach as res->count == 0 seems to be a valid case for a newly created database (please, correct me if I'm wrong).
Agree
I should have tried to run unit test before sending a patch
LS
On (30/08/16 17:07), Lukas Slebodnik wrote:
On (30/08/16 16:59), Fabiano Fidêncio wrote:
Lukaš,
On Tue, Aug 30, 2016 at 4:54 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
ehlo,
Clang static analyzer assume that ldb_search can found 0 entries in the tree "cn=sysdb". Thenvariable version could be used uninitialized.
We cannot get to such state in sssd but we already handle a case for more then one entry.
I don't think this is the right approach as res->count == 0 seems to be a valid case for a newly created database (please, correct me if I'm wrong).
Agree
I should have tried to run unit test before sending a patch
I looked deeper to the clang report And there are wrong assumption that output variable "version" is not initialized if function sysdb_cache_connect returns ERR_SYSDB_VERSION_TOO_OLD or ERR_SYSDB_VERSION_TOO_NEW
The reality is that output variable "version" is initialized especially for these two case.
It is a false positive but we might suppress the warning with initializing variable to NULL.
Attached is a gzipped html report from clang
LS
On Thu, Sep 1, 2016 at 5:12 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
On (30/08/16 17:07), Lukas Slebodnik wrote:
On (30/08/16 16:59), Fabiano Fidêncio wrote:
Lukaš,
On Tue, Aug 30, 2016 at 4:54 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
ehlo,
Clang static analyzer assume that ldb_search can found 0 entries in the tree "cn=sysdb". Thenvariable version could be used uninitialized.
We cannot get to such state in sssd but we already handle a case for more then one entry.
I don't think this is the right approach as res->count == 0 seems to be a valid case for a newly created database (please, correct me if I'm wrong).
Agree
I should have tried to run unit test before sending a patch
I looked deeper to the clang report And there are wrong assumption that output variable "version" is not initialized if function sysdb_cache_connect returns ERR_SYSDB_VERSION_TOO_OLD or ERR_SYSDB_VERSION_TOO_NEW
The reality is that output variable "version" is initialized especially for these two case.
It is a false positive but we might suppress the warning with initializing variable to NULL.
Indeed! Are you planning to submit this one-liner for review as well?
Best Regards,
On (01/09/16 17:22), Fabiano Fidêncio wrote:
On Thu, Sep 1, 2016 at 5:12 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
On (30/08/16 17:07), Lukas Slebodnik wrote:
On (30/08/16 16:59), Fabiano Fidêncio wrote:
Lukaš,
On Tue, Aug 30, 2016 at 4:54 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
ehlo,
Clang static analyzer assume that ldb_search can found 0 entries in the tree "cn=sysdb". Thenvariable version could be used uninitialized.
We cannot get to such state in sssd but we already handle a case for more then one entry.
I don't think this is the right approach as res->count == 0 seems to be a valid case for a newly created database (please, correct me if I'm wrong).
Agree
I should have tried to run unit test before sending a patch
I looked deeper to the clang report And there are wrong assumption that output variable "version" is not initialized if function sysdb_cache_connect returns ERR_SYSDB_VERSION_TOO_OLD or ERR_SYSDB_VERSION_TOO_NEW
The reality is that output variable "version" is initialized especially for these two case.
It is a false positive but we might suppress the warning with initializing variable to NULL.
Indeed! Are you planning to submit this one-liner for review as well?
I can.
LS
On Thu, Sep 1, 2016 at 6:02 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
On (01/09/16 17:22), Fabiano Fidêncio wrote:
On Thu, Sep 1, 2016 at 5:12 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
On (30/08/16 17:07), Lukas Slebodnik wrote:
On (30/08/16 16:59), Fabiano Fidêncio wrote:
Lukaš,
On Tue, Aug 30, 2016 at 4:54 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
ehlo,
Clang static analyzer assume that ldb_search can found 0 entries in the tree "cn=sysdb". Thenvariable version could be used uninitialized.
We cannot get to such state in sssd but we already handle a case for more then one entry.
I don't think this is the right approach as res->count == 0 seems to be a valid case for a newly created database (please, correct me if I'm wrong).
Agree
I should have tried to run unit test before sending a patch
I looked deeper to the clang report And there are wrong assumption that output variable "version" is not initialized if function sysdb_cache_connect returns ERR_SYSDB_VERSION_TOO_OLD or ERR_SYSDB_VERSION_TOO_NEW
The reality is that output variable "version" is initialized especially for these two case.
It is a false positive but we might suppress the warning with initializing variable to NULL.
Indeed! Are you planning to submit this one-liner for review as well?
I can.
Please, just fix a typo before pushing: (...) to NULL suppress" -> "(...) to NULL suppresses".
Acked-by: Fabiano Fid6encio fidencio@redhat.com
Best Regards,
On (01/09/16 18:15), Fabiano Fidêncio wrote:
On Thu, Sep 1, 2016 at 6:02 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
On (01/09/16 17:22), Fabiano Fidêncio wrote:
On Thu, Sep 1, 2016 at 5:12 PM, Lukas Slebodnik lslebodn@redhat.com wrote:
On (30/08/16 17:07), Lukas Slebodnik wrote:
On (30/08/16 16:59), Fabiano Fidêncio wrote:
Lukaš,
On Tue, Aug 30, 2016 at 4:54 PM, Lukas Slebodnik lslebodn@redhat.com wrote: > ehlo, > > Clang static analyzer assume that ldb_search can found > 0 entries in the tree "cn=sysdb". Thenvariable version > could be used uninitialized. > > We cannot get to such state in sssd but we already handle > a case for more then one entry.
I don't think this is the right approach as res->count == 0 seems to be a valid case for a newly created database (please, correct me if I'm wrong).
Agree
I should have tried to run unit test before sending a patch
I looked deeper to the clang report And there are wrong assumption that output variable "version" is not initialized if function sysdb_cache_connect returns ERR_SYSDB_VERSION_TOO_OLD or ERR_SYSDB_VERSION_TOO_NEW
The reality is that output variable "version" is initialized especially for these two case.
It is a false positive but we might suppress the warning with initializing variable to NULL.
Indeed! Are you planning to submit this one-liner for review as well?
I can.
Please, just fix a typo before pushing: (...) to NULL suppress" -> "(...) to NULL suppresses".
Nice catch Changed.
Acked-by: Fabiano Fid6encio fidencio@redhat.com
http://sssd-ci.duckdns.org/logs/job/52/94/summary.html
master: * 3f6aecfe5061e165c10829142854ec7189029407
LS
sssd-devel@lists.fedorahosted.org