| Message ID | 1494476871-26474-1-git-send-email-pushkar.iit@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2017-05-11 at 09:57 +0530, Pushkar Jambhlekar wrote: > Fixing Sparse warning. It should return bool, instead it returns > int. [] > diff --git a/fs/read_write.c b/fs/read_write.c [] > @@ -33,9 +33,9 @@ const struct file_operations generic_ro_fops = { > > EXPORT_SYMBOL(generic_ro_fops); > > -static inline int unsigned_offsets(struct file *file) > +static inline bool unsigned_offsets(struct file *file) > { > - return file->f_mode & FMODE_UNSIGNED_OFFSET; > + return !!(file->f_mode & FMODE_UNSIGNED_OFFSET); trivia: the !! isn't necessary as by definition all non-zero assigns to bool are converted to 1.
Should I change my implementation, i.e. remove '!!'? On Thu, May 11, 2017 at 10:09 AM, Joe Perches <joe@perches.com> wrote: > On Thu, 2017-05-11 at 09:57 +0530, Pushkar Jambhlekar wrote: >> Fixing Sparse warning. It should return bool, instead it returns >> int. > [] >> diff --git a/fs/read_write.c b/fs/read_write.c > [] >> @@ -33,9 +33,9 @@ const struct file_operations generic_ro_fops = { >> >> EXPORT_SYMBOL(generic_ro_fops); >> >> -static inline int unsigned_offsets(struct file *file) >> +static inline bool unsigned_offsets(struct file *file) >> { >> - return file->f_mode & FMODE_UNSIGNED_OFFSET; >> + return !!(file->f_mode & FMODE_UNSIGNED_OFFSET); > > trivia: the !! isn't necessary as by definition > all non-zero assigns to bool are converted to 1. >
On Thu, 2017-05-11 at 10:13 +0530, Pushkar Jambhlekar wrote:
> Should I change my implementation, i.e. remove '!!'?
That'd be up to Al.
At least one implementation using similar bit comparisons
in fs/*.c does not use !!
fs/locks.c:static bool lease_breaking(struct file_lock *fl)
fs/locks.c-{
fs/locks.c- return fl->fl_flags & (FL_UNLOCK_PENDING | FL_DOWNGRADE_PENDING)
fs/locks.c-}
I didn't look very hard.
If I remove '!!', sparse flags warning: fs/read_write.c:38:29: warning: incorrect type in return expression (different base types) fs/read_write.c:38:29: expected bool fs/read_write.c:38:29: got restricted fmode_t It means explicit conversion is needed. On Thu, May 11, 2017 at 10:25 AM, Joe Perches <joe@perches.com> wrote: > On Thu, 2017-05-11 at 10:13 +0530, Pushkar Jambhlekar wrote: >> Should I change my implementation, i.e. remove '!!'? > > That'd be up to Al. > > At least one implementation using similar bit comparisons > in fs/*.c does not use !! > > fs/locks.c:static bool lease_breaking(struct file_lock *fl) > fs/locks.c-{ > fs/locks.c- return fl->fl_flags & (FL_UNLOCK_PENDING | FL_DOWNGRADE_PENDING) > fs/locks.c-} > > I didn't look very hard.
On Thu, May 11, 2017 at 10:34:02AM +0530, Pushkar Jambhlekar wrote: > If I remove '!!', sparse flags warning: > > fs/read_write.c:38:29: warning: incorrect type in return expression > (different base types) > fs/read_write.c:38:29: expected bool > fs/read_write.c:38:29: got restricted fmode_t > > It means explicit conversion is needed. FVO"needed" equal to "needed to make sparse STFU"? If anything, that's sparse being wrong - evaluate.c:check_assignment_type() should do if (t == &ctype_bool) { if (is_fouled_type(s)) warning((*rp)->pos, "%s degrades to integer", show_typename(s->ctype.base_type)); goto Cast; } right after } else if (!(sclass & TYPE_RESTRICT)) goto Cast;
On Thu, May 11, 2017 at 8:07 AM, Al Viro <viro@zeniv.linux.org.uk> wrote: > FVO"needed" equal to "needed to make sparse STFU"? If anything, that's > sparse being wrong - evaluate.c:check_assignment_type() should do > if (t == &ctype_bool) { > if (is_fouled_type(s)) > warning((*rp)->pos, "%s degrades to integer", > show_typename(s->ctype.base_type)); > goto Cast; > } > right after > } else if (!(sclass & TYPE_RESTRICT)) > goto Cast; What about an explicit cast of restricted types to bool? I think we would want the equivalent of this patch for those too. -- Luc
diff --git a/fs/read_write.c b/fs/read_write.c index 47c1d44..d672830 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -33,9 +33,9 @@ const struct file_operations generic_ro_fops = { EXPORT_SYMBOL(generic_ro_fops); -static inline int unsigned_offsets(struct file *file) +static inline bool unsigned_offsets(struct file *file) { - return file->f_mode & FMODE_UNSIGNED_OFFSET; + return !!(file->f_mode & FMODE_UNSIGNED_OFFSET); } /**
Fixing Sparse warning. It should return bool, instead it returns int. Signed-off-by: Pushkar Jambhlekar <pushkar.iit@gmail.com> --- fs/read_write.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)