All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dominique Martinet <asmadeus@codewreck.org>
To: Christian Schoenebeck <linux_oss@crudebyte.com>
Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
	Eric Van Hensbergen <ericvh@kernel.org>,
	Latchesar Ionkov <lucho@ionkov.net>,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
	v9fs@lists.linux.dev
Subject: Re: [PATCH] fs/9p: Fix a datatype used with V9FS_DIRECT_IO
Date: Tue, 25 Apr 2023 19:40:22 +0900	[thread overview]
Message-ID: <ZEeuFlEAaARGqZol@codewreck.org> (raw)
In-Reply-To: <2755033.v0V8SJffbf@silver>

Christian Schoenebeck wrote on Tue, Apr 25, 2023 at 11:18:37AM +0200:
> > I'm surprised W=1 doesn't catch this... and now I'm checking higher
> > (noisy) W=, or even clang doesn't seem to print anything about e.g.
> > 'v9ses->flags & V9FS_DIRECT_IO is never true' or other warnings I'd have
> > expected to come up -- out of curiosity how did you find this?
> 
> Both gcc and clang only trigger an implicit conversion warning if the value of
> the expression can be evaluated at compile time (i.e. all operands are
> constant), then compiler realizes that the compile-time evaluated constant
> value is too big for the assignment destination and triggers the warning.

Right, `v9ses->flags = V9FS_DIRECT_IO` would have triggered it but not
with `|=` -- but in this case I was also expecting the check
`v9ses->flags & V9fs_DIRECT_IO` to flag something odd...
But nothing seems to care; testing with this snippet:
---
int foo(char x) {
	if (x & 0x200)
		return 1;
	return 0;
}
int foo2(unsigned char x) {
	if (x < 0)
		return 1;
	return 0;
}
---
gcc warns that the x < 0 is always false (clang actually doesn't, even
with scan-build, I must be missing a flag?), but I didn't find anything
complaining about the &.
I'd expect something like coverity to perform a bit better here but it's
a pain to use the "free for open source" version (... I just requested
access to https://scan.coverity.com/projects/128 but I have no idea if
they build next or not)

Oh, well; glad Christophe noticed anyway.

> > Would probably be interesting to run some form of the same in our
> > automation.
> 
> If there is any ATM? I als tried this issue with clang's undefined behaviour
> sanitizer and with the clang static analyzer. Both did not detect it.

There's at least the intel bot building with W=1 and warning if any new
such warning pops up (and I'd like to say I check myself, but I probably
forget about half the time; I looked at making W=1 default for our part
of the tree but it didn't look trivial? I'll try to have another look);
but I'm not aware of anyone testing with scan-build or something else
that'd contact us on new defects.

-- 
Dominique Martinet | Asmadeus

  reply	other threads:[~2023-04-25 10:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-25  6:47 [PATCH] fs/9p: Fix a datatype used with V9FS_DIRECT_IO Christophe JAILLET
2023-04-25  7:08 ` Dominique Martinet
2023-04-25  9:18   ` Christian Schoenebeck
2023-04-25 10:40     ` Dominique Martinet [this message]
2023-04-25 12:19       ` Christian Schoenebeck
2023-04-25 13:14   ` Dan Carpenter
2023-04-26  0:35     ` Dominique Martinet

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=ZEeuFlEAaARGqZol@codewreck.org \
    --to=asmadeus@codewreck.org \
    --cc=christophe.jaillet@wanadoo.fr \
    --cc=ericvh@kernel.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=v9fs@lists.linux.dev \
    /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.