Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201026213040.3889546-1-arnd@kernel.org> <20201027035558.16864-1-xie.he.0141@gmail.com>
 <CAJht_EPSs6W-r6kpWUNQDPzCjL-+_8mqq2JBoY=qhsQREgn92g@mail.gmail.com>
In-Reply-To: <CAJht_EPSs6W-r6kpWUNQDPzCjL-+_8mqq2JBoY=qhsQREgn92g@mail.gmail.com>
From:   Arnd Bergmann <arnd@kernel.org>
Date:   Tue, 27 Oct 2020 14:23:46 +0100
Message-ID: <CAK8P3a3JTg5Mi2XC9AEC+YwH552M_TXDY4BaULZz5WmEb3woRQ@mail.gmail.com>
Subject: Re: [PATCH net-next 01/11] atm: horizon: shut up clang null pointer
 arithmetic warning
To:     Xie He <xie.he.0141@gmail.com>
Cc:     Chas Williams <3chas3@gmail.com>,
        Nathan Chancellor <natechancellor@gmail.com>,
        Nick Desaulniers <ndesaulniers@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        linux-atm-general@lists.sourceforge.net,
        Linux Kernel Network Developers <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        clang-built-linux <clang-built-linux@googlegroups.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Oct 27, 2020 at 5:02 AM Xie He <xie.he.0141@gmail.com> wrote:
>
> On Mon, Oct 26, 2020 at 8:56 PM Xie He <xie.he.0141@gmail.com> wrote:
> >
> > > -  for (mem = (HDW *) memmap; mem < (HDW *) (memmap + 1); ++mem)
> > > +  for (mem = (HDW *) memmap; mem < (HDW *) ((uintptr_t)memmap + 1); ++mem)
> >
> > Note that these two lines are semantically different. In the first line,
> > "+ 1" moves the pointer by (sizeof memmap) bytes. However in the second
> > line, "+ 1" moves the pointer by only 1 byte.
>
> Correction: in the first line "+ 1" moves the pointer by (sizeof *memmap) bytes.

Ah, of course. I had looked up the types but mixed up the memmap
and HDW definitions, but then got confused trying to understand the
logic in wr_mem() that operates on bytes but expands them into
multiples of 4.

I've modified it as below now, will resend along with the other patches
if you think this makes sense.

        Arnd

--- a/drivers/atm/horizon.c
+++ b/drivers/atm/horizon.c
@@ -1815,7 +1815,7 @@ static int hrz_init(hrz_dev *dev)

   int buff_count;

-  HDW * mem;
+  uintptr_t offset;

   cell_buf * tx_desc;
   cell_buf * rx_desc;
@@ -1841,8 +1841,8 @@ static int hrz_init(hrz_dev *dev)

   printk (" clearing memory");

-  for (mem = (HDW *) memmap; mem < (HDW *) (memmap + 1); ++mem)
-    wr_mem (dev, mem, 0);
+  for (offset = 0; offset < sizeof(struct MEMMAP); offset++)
+    wr_mem (dev, (HDW *)offset, 0);

   printk (" tx channels");