Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 3 May 2023 17:44:29 +0900
From:   Dominique Martinet <asmadeus@codewreck.org>
To:     Christian Schoenebeck <linux_oss@crudebyte.com>
Cc:     Eric Van Hensbergen <ericvh@gmail.com>,
        Latchesar Ionkov <lucho@ionkov.net>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Simon Horman <simon.horman@corigine.com>, v9fs@lists.linux.dev,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH v2 5/5] 9p: remove dead stores (variable set again
 without being read)
Message-ID: <ZFIe7dLEncWN5YaQ@codewreck.org>
References: <20230427-scan-build-v2-0-bb96a6e6a33b@codewreck.org>
 <20230427-scan-build-v2-5-bb96a6e6a33b@codewreck.org>
 <3207385.lLoMtQYYpd@silver>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <3207385.lLoMtQYYpd@silver>
Precedence: bulk

Christian Schoenebeck wrote on Wed, May 03, 2023 at 10:22:46AM +0200:
> On Wednesday, May 3, 2023 9:49:29 AM CEST Dominique Martinet wrote:
> > The 9p code for some reason used to initialize variables outside of the
> > declaration, e.g. instead of just initializing the variable like this:
> > 
> > int retval = 0
> > 
> > We would be doing this:
> > 
> > int retval;
> > retval = 0;
> 
> OK, but AFAICS this patch would simply remove all initializations. I would
> expect at least a default initialization at variable declaration instead.

Yes, clang doesn't seem to complain about 'int reval = 0' so the patch
can just be updated to do that instead; I just removed them because the
sheer number made it faster to do that.
Happy to drop this last patch for now and rework it when time permits.

> > This is perfectly fine and the compiler will just optimize dead stores
> > anyway, but scan-build seems to think this is a problem and there are
> > many of these warnings making the output of scan-build full of such
> > warnings:
> > fs/9p/vfs_inode.c:916:2: warning: Value stored to 'retval' is never read [deadcode.DeadStores]
> >         retval = 0;
> >         ^        ~
> 
> Honestly I don't see much value in this warning. Can't we just disable this
> warning for 9p code or is this just controllable for the entire project?

Dead stores in itself is a useful warning, it's what found the real bug
where return value was lost in patch 1 of this series, I don't think we
should just disable the warning.

-- 
Dominique Martinet | Asmadeus