Fixing fmode_t warnings

Fixing fmode_t warnings

[Steve French]

Do you think it is worthwhile fixing this "sparse" warning ?  oflags
is an int since the lookup intent open.flags is an int ...

CHECK   fs/cifs/dir.c
fs/cifs/dir.c:169:16: warning: restricted fmode_t degrades to integer
fs/cifs/dir.c:171:16: warning: restricted fmode_t degrades to integer

                int oflags = nd->intent.open.flags;
                desiredAccess = 0;
(line 169) if (oflags & FMODE_READ)
                        desiredAccess |= GENERIC_READ;
(line 171) if (oflags & FMODE_WRITE) {


-- 
Thanks,

Steve

Re: Fixing fmode_t warnings

[Steve French]

The same kind of thing causes a sparse warning in fs/open.c (and
fs/namei.c and fs/locks.c too)

CHECK   fs/open.c
fs/open.c:270:29: warning: incorrect type in argument 2 (different base types)
fs/open.c:270:29:    expected unsigned int [unsigned] mode
fs/open.c:270:29:    got restricted fmode_t [usertype] <noident>

fs/open.c:	error = break_lease(inode, FMODE_WRITE);

On Tue, Nov 18, 2008 at 10:41 AM, Steve French <smfrench@gmail.com> wrote:
> Do you think it is worthwhile fixing this "sparse" warning ?  oflags
> is an int since the lookup intent open.flags is an int ...
>
> CHECK   fs/cifs/dir.c
> fs/cifs/dir.c:169:16: warning: restricted fmode_t degrades to integer
> fs/cifs/dir.c:171:16: warning: restricted fmode_t degrades to integer
>
>                int oflags = nd->intent.open.flags;
>                desiredAccess = 0;
> (line 169) if (oflags & FMODE_READ)
>                        desiredAccess |= GENERIC_READ;
> (line 171) if (oflags & FMODE_WRITE) {
>
>
> --
> Thanks,
>
> Steve
>



-- 
Thanks,

Steve

Re: Fixing fmode_t warnings

[Al Viro]

On Tue, Nov 18, 2008 at 10:41:17AM -0600, Steve French wrote:
> Do you think it is worthwhile fixing this "sparse" warning ?  oflags
> is an int since the lookup intent open.flags is an int ...
> 
> CHECK   fs/cifs/dir.c
> fs/cifs/dir.c:169:16: warning: restricted fmode_t degrades to integer
> fs/cifs/dir.c:171:16: warning: restricted fmode_t degrades to integer
> 
>                 int oflags = nd->intent.open.flags;
>                 desiredAccess = 0;
> (line 169) if (oflags & FMODE_READ)
>                         desiredAccess |= GENERIC_READ;
> (line 171) if (oflags & FMODE_WRITE) {

It's worse than that; open.flags *mixes* FMODE_... with O_...
The right solution is to get rid of the damn thing and pass struct file
with pre-fill ->f_mode to (split-off) variant of create.

Re: Fixing fmode_t warnings

[Jeff Layton]

On Tue, 18 Nov 2008 10:41:17 -0600
"Steve French" <smfrench@gmail.com> wrote:

> Do you think it is worthwhile fixing this "sparse" warning ?  oflags
> is an int since the lookup intent open.flags is an int ...
> 
> CHECK   fs/cifs/dir.c
> fs/cifs/dir.c:169:16: warning: restricted fmode_t degrades to integer
> fs/cifs/dir.c:171:16: warning: restricted fmode_t degrades to integer
> 
>                 int oflags = nd->intent.open.flags;
>                 desiredAccess = 0;
> (line 169) if (oflags & FMODE_READ)
>                         desiredAccess |= GENERIC_READ;
> (line 171) if (oflags & FMODE_WRITE) {
> 
> 

In general, I'm for fixing most warnings unless there is a good reason
not to. When things compile and checkers run cleanly it can really help
you to detect bugs...

My suggestion would be to go ahead and make oflags a fmode_t and just
cast the intent.open.flags to fmode_t.

-- 
Jeff Layton <jlayton@redhat.com>

Re: Fixing fmode_t warnings

[Dave Kleikamp]

On Tue, 2008-11-18 at 12:03 -0500, Jeff Layton wrote:
> On Tue, 18 Nov 2008 10:41:17 -0600
> "Steve French" <smfrench@gmail.com> wrote:
> 
> > Do you think it is worthwhile fixing this "sparse" warning ?  oflags
> > is an int since the lookup intent open.flags is an int ...
> > 
> > CHECK   fs/cifs/dir.c
> > fs/cifs/dir.c:169:16: warning: restricted fmode_t degrades to integer
> > fs/cifs/dir.c:171:16: warning: restricted fmode_t degrades to integer
> > 
> >                 int oflags = nd->intent.open.flags;
> >                 desiredAccess = 0;
> > (line 169) if (oflags & FMODE_READ)
> >                         desiredAccess |= GENERIC_READ;
> > (line 171) if (oflags & FMODE_WRITE) {
> > 
> > 
> 
> In general, I'm for fixing most warnings unless there is a good reason
> not to. When things compile and checkers run cleanly it can really help
> you to detect bugs...
> 
> My suggestion would be to go ahead and make oflags a fmode_t and just
> cast the intent.open.flags to fmode_t.

I agree with Al that mixing O_* flags and FMODE_* flags in the same
field is broken.  I'd just as well leave the warning until a real
cleanup can separate the two.  There are other places that simply
changing the type isn't going to work.
-- 
David Kleikamp
IBM Linux Technology Center