xfsprogs: fix wrong variable type in write_once function

PATCH Superseded Zorro Lang 2 participants 4 messages Nov 10, 2017

lore

Share & tools

lore

Zorro Lang

author

2017-11-10 01:17 UTC #0 lore raw

PATCH 1/1 xfsprogs: fix wrong variable type in write_once function

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);

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
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.

Signed-off-by Zorro Lang <[email protected]>

---
 io/pwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

io/pwrite.c +1 -1

index 26f79579..b28b6039 100644
@@ -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;

Signature · 1 line

2.13.6

Eric Sandeen

2017-11-10 01:47 UTC #1 lore raw

Quoted 10 lines · Zorro Lang

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);
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
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

Quoted 24 lines

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 <[email protected]>
---
 io/pwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
io/pwrite.c +1 -1
index 26f79579..b28b6039 100644
@@ -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;

Zorro Lang

author

2017-11-10 02:06 UTC #2 lore raw

Quoted 5 lines · Eric Sandeen

Quoted 4 lines · Zorro Lang
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);

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
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);

Quoted 7 lines

static int
do_pwrite(
...
as does do_pwritev, which it calls:

Quoted 7 lines

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

Quoted 32 lines

Thanks,
-Eric
Quoted 16 lines
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 <[email protected]>
---
 io/pwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
io/pwrite.c +0 -0
index 26f79579..b28b6039 100644
io/pwrite.c write_once( · line 275
Quoted 8 lines
io/pwrite.c +1 -1
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Eric Sandeen

2017-11-10 02:08 UTC #3 lore raw

Quoted 8 lines · Zorro Lang

Quoted 7 lines · Eric Sandeen
Quoted 6 lines · Zorro Lang
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);
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
CID 1420710: Control flow issues (NO_EFFECT)
This less-than-zero comparison of an unsigned value is never true. "bytes < 0UL".

Quoted 8 lines

Quoted 5 lines
Quoted 4 lines
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