[SSSD] [PATCH] Suppress warning maybe-uninitialized

Sumit Bose sbose at redhat.com
Thu Oct 2 20:51:31 UTC 2014


On Thu, Oct 02, 2014 at 10:20:56PM +0200, Lukas Slebodnik wrote:
> On (02/10/14 15:12), Simo Sorce wrote:
> >On Thu, 2 Oct 2014 19:33:02 +0200
> >Lukas Slebodnik <lslebodn at redhat.com> wrote:
> >
> >> ehlo,
> >> 
> >> There is a warning with some version of gcc (4.9.1, 4.4.7).
> >> Warning is not visible with gcc 4.8
> >> In my opinion, variable "a" should be initialized every time if
> >> "maps" is not NULL.
> >
> >I agree that "a" should be intialized every time, in fact I do not
> >agree with your patch where you set it to be a volatile, it is not.
> >
> >The main issue here is that the name and way this variable is used is
> >confusing.
> >
> >A better way would be to define a locally in the first loop, and in the
> >function define a better name initialized to a negative value.
> >
> >the name should probably be called something like base_attr_idx and
> >intialized as:
> >
> >int base_attr_idx = 0;
> >
> >and if the first part of the code finds the base_attr it is looking for
> >it would simply assing a to it (note we do away with "a" completely).
> >
> >if (map) {
> >    for (i = 1; i < attrs_num; i++) {
> >       ...
> >    }
> >    if (i < attrs_num) {
> >        store = true;
> >        base_attr_idx = i;
> >        ...
> >    }
> >}
> >
> >then the second for loop where you get the error would be change to:
> >for (ai = base_attr_idx; ai > 0 && ai < attrs_num; ai++) {
> >
> I suppose you meant similar patch
> diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
> index 7034b11..6e8b16e 100644
> --- a/src/providers/ldap/sdap.c
> +++ b/src/providers/ldap/sdap.c
> @@ -305,6 +305,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
>      char *str;
>      int lerrno;
>      int a, i, ret, ai;
> +    int base_attr_idx;
>      const char *name;
>      bool store;
>      bool base64;
> @@ -412,6 +413,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
>              /* interesting attr */
>              if (a < attrs_num) {
>                  store = true;
> +                base_attr_idx = a;
>                  name = map[a].sys_name;
>                  if (strcmp(name, SYSDB_SSH_PUBKEY) == 0) {
>                      base64 = true;
> @@ -478,7 +480,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
>                           * attrs in case there is a map. Find all that match
>                           * and copy the value
>                           */
> -                        for (ai = a; ai < attrs_num; ai++) {
> +                        for (ai = base_attr_idx; ai < attrs_num; ai++) {
>                              /* check if this attr is valid with the chosen
>                               * schema */
>                              if (!map[ai].name) continue;
> 
> 
> It didn't help.
> src/providers/ldap/sdap.c: In function 'sdap_parse_entry':
> src/providers/ldap/sdap.c:308:9: error: 'base_attr_idx' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>      int base_attr_idx;
>          ^
> cc1: all warnings being treated as errors
> (BTW: There is no difference between using a and base_attr_idx)
> 
> Any other better solution is welcomed.

Just a wild guess. Maybe the compiler is doing some wild optimizations
assuming that attrs_num might be 1, 0 or even smaller. Does it help to
check for (map && num_attrs > 1) ? (I would try myself, but I do not
have the right gcc version at hand)

bye,
Sumit

> 
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel



More information about the sssd-devel mailing list