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:   Tue, 25 Apr 2023 20:13:27 +0900
From:   asmadeus@codewreck.org
To:     Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc:     Eric Van Hensbergen <ericvh@kernel.org>,
        v9fs-developer@lists.sourceforge.net,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux_oss@crudebyte.com
Subject: Re: [PATCH v5] fs/9p: remove writeback fid and fix per-file modes
Message-ID: <ZEe11w5mG_pv5X_o@codewreck.org>
References: <20230218003323.2322580-11-ericvh@kernel.org>
 <ZCEGmS4FBRFClQjS@7e9e31583646>
 <7686c810-4ed6-9e3a-3714-8b803e2d3c46@wanadoo.fr>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <7686c810-4ed6-9e3a-3714-8b803e2d3c46@wanadoo.fr>
Precedence: bulk

Christophe JAILLET wrote on Tue, Apr 25, 2023 at 09:11:01AM +0200:
> This code looks strange.
> P9_OWRITE is 0x01, so !P9_OWRITE is 0.
> So the code is equivalent to "p9_omode = P9_ORDWR;"
> 
> Is it what is expexted?
> 
> Maybe
> 	p9_omode = (p9_omode & ~P9_OWRITE) | P9_ORDWR;
> ?

Since we're discussing tooling, sparse caught this one:
fs/9p/vfs_inode.c:826:38: warning: dubious: x & !y
fs/9p/vfs_inode_dotl.c:291:38: warning: dubious: x & !y

(runing with make `M=fs/9p W=1 C=2` ; unfortunately error code doesn't
reflect a problem so that'll require inspecting output to automate...)


I've tried running scan-build in a very old-fashioned way (getting the
gcc lines from make V=1 and re-running with scan-build) and it had some
arguable warnings (setting `ret = 0` before it is overwritten again is
considered a dead store), but it had some real problems as well so it
might be worth fixing these just to reduce the clutter and run it all
the time.
I'll post a couple of patches tomorrow (unrelated to this)

-- 
Dominique