[PATCH 2/2] soc: qcom: cmd-db: Fix compiler warnings

[bjorn.andersson@linaro.org (Bjorn Andersson)]

On Mon 25 Jun 14:08 PDT 2018, Arnd Bergmann wrote:

> On Mon, Jun 25, 2018 at 10:54 PM, Andy Gross <andy.gross@linaro.org> wrote:
> > On Mon, Jun 25, 2018 at 10:30:17PM +0200, Arnd Bergmann wrote:
> >> On Mon, Jun 25, 2018 at 7:05 PM, Andy Gross <andy.gross@linaro.org> wrote:
> >>
> >> I generally don't like patches that add incorrect initializations to
> >> work around warnings like this.
> >
> > Correct me if I am wrong, but this just masks the warnings on toolchain/
> > platforms that exhibit these warnings.  I'm not entirely fond of doing
> > blanket init either to make the warnings go away either...
> 
> My patch disallows the case where the compiler can see that the variable
> is actually used without an initialization, and that code wouldn't
> work properly.
> 
> The problem is that in this loop:
> 
>         for (i = 0; i < MAX_SLV_ID; i++) {
>                 rsc_hdr = &cmd_db_header->header[i];
>                 if (!rsc_hdr->slv_id)
>                         break;
> 
>                 ent = rsc_to_entry_header(rsc_hdr);
>                 for (j = 0; j < le16_to_cpu(rsc_hdr->cnt); j++, ent++) {
>                         if (memcmp(ent->id, query, sizeof(ent->id)) == 0)
>                                 break;
>                 }
> 
>                 if (j < le16_to_cpu(rsc_hdr->cnt)) {
>                         memcpy(eh, ent, sizeof(*ent));
>                         memcpy(rh, rsc_hdr, sizeof(*rh));
>                         return 0;
>                 }
>         }
> 
> I think gcc sees that cmd_db_header is initialized to NULL and never
> written to after of_reserved_mem_lookup() returns NULL during
> cmd_db_dev_probe().
> 
> This means we are that cmd_db_get_header never intializes the
> resulting structure. It also never returns zero, but gcc doesn't
> see that here.
> 

The optimization that I can see would lead to a difference here is if
the conditional in cmd_db_read() is just removed as the compiler see it
always being true, but then doesn't propagate this to the error handling
in cmd_db_get_header(); but that wouldn't make much sense...

Otherwise I think cmd_db_header would be equally uninitialized in the
case where CONFIG_OF_RESERVED_MEM is set, but we haven't seen any issues
related to this on arm64.


Unless there's something I'm missing I would prefer that we just take
Anders' proposed fix, it does mask the problem in a more direct way.

Regards,
Bjorn