| Message ID | 20171110011737.4089-1-zlang@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 11/9/17 7:17 PM, 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, An int, actually, right? static int do_pwrite( ... as does do_pwritev, which it calls: static int do_pwritev( ... Can you chase through the types a bit more for consistency while you're in here? Thanks, -Eric > 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. > > Signed-off-by: Zorro Lang <zlang@redhat.com> > --- > io/pwrite.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/io/pwrite.c b/io/pwrite.c > index 26f79579..b28b6039 100644 > --- a/io/pwrite.c > +++ b/io/pwrite.c > @@ -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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 09, 2017 at 07:47:14PM -0600, Eric Sandeen wrote: > On 11/9/17 7:17 PM, 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, > > An int, actually, right? Yeah, I mean the pwrite manpage defines: ssize_t pwrite(int fd, const void *buf, size_t count, off_t offset); > > static int > do_pwrite( > ... > > as does do_pwritev, which it calls: > > static int > do_pwritev( > > ... > > Can you chase through the types a bit more for consistency while you're > in here? Sure, I'll go through the do_pwrite(). Thanks, Zorro > > Thanks, > -Eric > > > 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. > > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > --- > > io/pwrite.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/io/pwrite.c b/io/pwrite.c > > index 26f79579..b28b6039 100644 > > --- a/io/pwrite.c > > +++ b/io/pwrite.c > > @@ -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@vger.kernel.org > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/9/17 8:06 PM, Zorro Lang wrote: > On Thu, Nov 09, 2017 at 07:47:14PM -0600, Eric Sandeen wrote: >> On 11/9/17 7:17 PM, 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, >> An int, actually, right? > Yeah, I mean the pwrite manpage defines: > > ssize_t pwrite(int fd, const void *buf, size_t count, off_t offset); yes, but you said "do_pwrite" - which does call pwrite (ssize_t), and returns it through an int. :) hence the request for a bit more fixing. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/io/pwrite.c b/io/pwrite.c index 26f79579..b28b6039 100644 --- a/io/pwrite.c +++ b/io/pwrite.c @@ -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;