Re: nfs_super_set_maxbytes patch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 18 Mar 2016 at 8:55, Alexey Dvoichenkov wrote:

> On Thu, 17 Mar 2016 09:20:14 -0400
> Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> wrote:
> 
> > > --- fs/nfs/internal.h.orig      2015-11-02 10:05:25.000000000 +1000
> > > +++ fs/nfs/internal.h   2016-01-02 03:19:04.599120855 +1000
> > > @@ -612,9 +612,9 @@
> > >  static inline
> > >  void nfs_super_set_maxbytes(struct super_block *sb, __u64 maxfilesize)
> > >  {
> > > +       if (maxfilesize > MAX_LFS_FILESIZE || maxfilesize == 0)
> > > +               maxfilesize = MAX_LFS_FILESIZE;
> > >         sb->s_maxbytes = (loff_t)maxfilesize;
> > > -       if (sb->s_maxbytes > MAX_LFS_FILESIZE || sb->s_maxbytes <= 0)
> > > -               sb->s_maxbytes = MAX_LFS_FILESIZE;
> > >  }
> > >
> > 
> > Why are we having to change _correct_ code in order to work with a
> > checking tool?

first of all, it's correct in that it relies on implementation defined
behaviour and not UB. C99 6.3.1.3 says:

    Otherwise, the new type is signed and the value cannot be represented
    in it; either the result is implementation-defined or an implementation-
    defined signal is raised.

it's still unfortunate of course as the reliance on the above is trivially
avoidable (and would also satisfy the size overflow plugin which otherwise
chooses the other possible behaviour). all that said, i think the bigger
problem with this code pattern is that it's putting the cart before the
horses so to speak, so it'd be better if it was changed for that reason
alone.

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux