All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Andy Gross <andy.gross@linaro.org>
Cc: Anders Roxell <anders.roxell@linaro.org>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-arm-msm@vger.kernel.org,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	mahesh sivasubramanian <msivasub@codeaurora.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 2/2] soc: qcom: cmd-db: Fix compiler warnings
Date: Mon, 25 Jun 2018 23:08:40 +0200	[thread overview]
Message-ID: <CAK8P3a1+qs3f2cLPKPiMKR4L74dwpAkBcZPNVG4hk_nfx12Xqw@mail.gmail.com> (raw)
In-Reply-To: <20180625205406.GC4689@hector.attlocal.net>

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.

A slightly more drastic workaround might be to initialize *eh
at the start of cmd_db_get_header().

     Arnd

WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] soc: qcom: cmd-db: Fix compiler warnings
Date: Mon, 25 Jun 2018 23:08:40 +0200	[thread overview]
Message-ID: <CAK8P3a1+qs3f2cLPKPiMKR4L74dwpAkBcZPNVG4hk_nfx12Xqw@mail.gmail.com> (raw)
In-Reply-To: <20180625205406.GC4689@hector.attlocal.net>

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.

A slightly more drastic workaround might be to initialize *eh
at the start of cmd_db_get_header().

     Arnd

  reply	other threads:[~2018-06-25 21:08 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-25 17:05 [PATCH 1/2] soc: qcom: cmd-db: Fix incorrect errno checks Andy Gross
2018-06-25 17:05 ` Andy Gross
2018-06-25 17:05 ` [PATCH 2/2] soc: qcom: cmd-db: Fix compiler warnings Andy Gross
2018-06-25 17:05   ` Andy Gross
2018-06-25 19:44   ` Andy Gross
2018-06-25 19:44     ` Andy Gross
2018-06-25 20:30   ` Arnd Bergmann
2018-06-25 20:30     ` Arnd Bergmann
2018-06-25 20:54     ` Andy Gross
2018-06-25 20:54       ` Andy Gross
2018-06-25 21:08       ` Arnd Bergmann [this message]
2018-06-25 21:08         ` Arnd Bergmann
2018-06-25 23:26         ` Bjorn Andersson
2018-06-25 23:26           ` Bjorn Andersson
2018-06-25 19:14 ` [PATCH 1/2] soc: qcom: cmd-db: Fix incorrect errno checks Bjorn Andersson
2018-06-25 19:14   ` Bjorn Andersson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAK8P3a1+qs3f2cLPKPiMKR4L74dwpAkBcZPNVG4hk_nfx12Xqw@mail.gmail.com \
    --to=arnd@arndb.de \
    --cc=anders.roxell@linaro.org \
    --cc=andy.gross@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=msivasub@codeaurora.org \
    --cc=sboyd@kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.