On Sat, Nov 11, 2017 at 09:46:48AM -0600, Eric Sandeen wrote:
> On 11/11/17 7:57 AM, Zorro Lang wrote:
> > The 'Coverity Scan' found a problem in new write_once() function:
> >
> > 272 size_t bytes;
> > 273 bytes = do_pwrite(file->fd, offset, count, count, pwritev2_flags);
> >>>> CID 1420710: Control flow issues (NO_EFFECT)
> >>>> This less-than-zero comparison of an unsigned value is never true. "bytes < 0UL".
> > 274 if (bytes < 0)
> > 275 return -1;
> >
> > That's unreasonable. do_pwrite return 'ssize_t' type value, which can
> > be less than zero, but we use a 'size_t' to get the return value. So
> > change the size_t to ssize_t for it can store the return value
> > correctly.
> >
> > By the chance, correct all 'ssize_t' type problems in pwrite related
> > functions.
> >
> > Signed-off-by: Zorro Lang <zlang@xxxxxxxxxx>
> > ---
> >
> > V2 changed more than V1, as below:
> > 1) The return value type of do_pwritev and do_pwrite, from 'int' to 'ssize_t'
> > 2) The arguments type of do_pwritev and do_pwrite, from 'ssize_t' to 'size_t'
> >
> > Thanks,
> > Zorro
> >
> > io/pwrite.c | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/io/pwrite.c b/io/pwrite.c
> > index 26f79579..3df976a8 100644
> > --- a/io/pwrite.c
> > +++ b/io/pwrite.c
> > @@ -61,12 +61,12 @@ pwrite_help(void)
> > }
> >
> > #ifdef HAVE_PWRITEV
> > -static int
> > +static ssize_t
> > do_pwritev(
> > int fd,
> > off64_t offset,
> > - ssize_t count,
> > - ssize_t buffer_size,
> > + size_t count,
> > + size_t buffer_size,
>
> I had not noticed this before, but this is odd; buffer_size is not used
> in this function. Instead, we test the global "buffersize"
>
> I'm not sure what's going on there, need to read the code bit more.
> That's probably a separate fix though.
The buffer_size argument in do_pwritev/do_preadv is useless. I'll remove
it in a separate patch. But due to this patch changed the 'buffer_size'
place, I'll send this patch and 'remove buffer_size' patch together.
To prevent merge conflict.
Thanks,
Zorro
>
> I think the rest looks ok,
>
> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
>
>
>
> > int pwritev2_flags)
> > {
> > int vecs = 0;
> > @@ -105,12 +105,12 @@ do_pwritev(
> > #define do_pwritev(fd, offset, count, buffer_size) (0)
> > #endif
> >
> > -static int
> > +static ssize_t
> > do_pwrite(
> > int fd,
> > off64_t offset,
> > - ssize_t count,
> > - ssize_t buffer_size,
> > + size_t count,
> > + size_t buffer_size,
> > int pwritev2_flags)
> > {
> > if (!vectors)
> > @@ -269,7 +269,7 @@ write_once(
> > long long *total,
> > int pwritev2_flags)
> > {
> > - size_t bytes;
> > + ssize_t bytes;
> > bytes = do_pwrite(file->fd, offset, count, count, pwritev2_flags);
> > if (bytes < 0)
> > return -1;
>
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
