On Sun, Oct 08, 2023 at 08:21:45AM +0800, Xiubo Li wrote:
>
> On 10/7/23 16:52, Dan Carpenter wrote:
> > In this code "ret" is type long and "src_objlen" is unsigned int. The
> > problem is that on 32bit systems, when we do the comparison signed longs
> > are type promoted to unsigned int. So negative error codes from
> > do_splice_direct() are treated as success instead of failure.
> >
> > Fixes: 1b0c3b9f91f0 ("ceph: re-org copy_file_range and fix some error paths")
> > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > ---
> > 32bit is so weird and ancient. It's strange to think that unsigned int
> > has more positive bits than signed long.
> >
> > fs/ceph/file.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > index b1da02f5dbe3..b5f8038065d7 100644
> > --- a/fs/ceph/file.c
> > +++ b/fs/ceph/file.c
> > @@ -2969,7 +2969,7 @@ static ssize_t __ceph_copy_file_range(struct file *src_file, loff_t src_off,
> > ret = do_splice_direct(src_file, &src_off, dst_file,
> > &dst_off, src_objlen, flags);
> > /* Abort on short copies or on error */
> > - if (ret < src_objlen) {
> > + if (ret < (long)src_objlen) {
> > dout("Failed partial copy (%zd)\n", ret);
> > goto out;
> > }
>
> Good catch and makes sense to me.
>
Thanks.
> I also ran a test in 64bit system, the output is the same too:
>
> int x = -1
> unsigned int y = 2
> x > y
Here none of the types are int. It's long and unsigned int. So how
type promotion works (normally, there are also weird exceptions like ?:
and <<) is when you have two variables then you by default at least type
promote both sides to int. But if one side is larger than int, then you
type promote it to which ever has more positive bits.
regards,
dan carpenter
