xfsprogs: fix wrong variable type in write_once function

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".

In io/pwrite.c, if not define HAVE_PWRITEV, we will use:
#define do_pwritev(fd, offset, count, buffer_size) (0)

But the real do_pwritev() function is:
do_pwritev(fd, offset, count, buffer_size, pwritev2_flags);

There's one more 'pwritev2_flags' argument.

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

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

io/pwrite.c +1 -1

index 3df976a8..a89edfd0 100644
@@ -102,7 +102,7 @@ do_pwritev(
 	return bytes;
 }
 #else
-#define do_pwritev(fd, offset, count, buffer_size) (0)
+#define do_pwritev(fd, offset, count, buffer_size, pwritev2_flags) (0)
 #endif
 
 static ssize_t

do_preadv and do_pwritev all have a 'buffer_size' argument, but they never used it. Instead of it, they use global 'buffersize' variable, which is initialized in alloc_buffer(). As the 'buffer_size' is useless, so remove it for clear code.

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

---
 io/pread.c  | 7 +++----
 io/pwrite.c | 5 ++---
 2 files changed, 5 insertions(+), 7 deletions(-)

io/pread.c +3 -4

index 60650aa3..98e992b0 100644
@@ -176,8 +176,7 @@ static ssize_t
 do_preadv(
 	int		fd,
 	off64_t		offset,
-	size_t		count,
-	size_t		buffer_size)
+	size_t		count)
 {
 	int		vecs = 0;
 	ssize_t		oldlen = 0;

@@ -205,7 +204,7 @@ do_preadv(
 	return bytes;
 }
 #else
-#define do_preadv(fd, offset, count, buffer_size) (0)
+#define do_preadv(fd, offset, count) (0)
 #endif
 
 static ssize_t

@@ -218,7 +217,7 @@ do_pread(
 	if (!vectors)
 		return pread(fd, buffer, min(count, buffer_size), offset);
 
-	return do_preadv(fd, offset, count, buffer_size);
+	return do_preadv(fd, offset, count);
 }
 
 static int

io/pwrite.c +2 -3

index a89edfd0..f75c6164 100644
@@ -66,7 +66,6 @@ do_pwritev(
 	int		fd,
 	off64_t		offset,
 	size_t		count,
-	size_t		buffer_size,
 	int 		pwritev2_flags)
 {
 	int vecs = 0;

@@ -102,7 +101,7 @@ do_pwritev(
 	return bytes;
 }
 #else
-#define do_pwritev(fd, offset, count, buffer_size, pwritev2_flags) (0)
+#define do_pwritev(fd, offset, count, pwritev2_flags) (0)
 #endif
 
 static ssize_t

@@ -116,7 +115,7 @@ do_pwrite(
 	if (!vectors)
 		return pwrite(fd, buffer, min(count, buffer_size), offset);
 
-	return do_pwritev(fd, offset, count, buffer_size, pwritev2_flags);
+	return do_pwritev(fd, offset, count, pwritev2_flags);
 }
 
 static int

do_preadv and do_pwritev all have a 'buffer_size' argument, but they never used it. Instead of it, they use global 'buffersize' variable, which is initialized in alloc_buffer(). As the 'buffer_size' is useless, so remove it for clear code.

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

---
 io/pread.c  | 7 +++----
 io/pwrite.c | 5 ++---
 2 files changed, 5 insertions(+), 7 deletions(-)

io/pread.c +3 -4

index 60650aa3..98e992b0 100644
@@ -176,8 +176,7 @@ static ssize_t
 do_preadv(
 	int		fd,
 	off64_t		offset,
-	size_t		count,
-	size_t		buffer_size)
+	size_t		count)
 {
 	int		vecs = 0;
 	ssize_t		oldlen = 0;

@@ -205,7 +204,7 @@ do_preadv(
 	return bytes;
 }
 #else
-#define do_preadv(fd, offset, count, buffer_size) (0)
+#define do_preadv(fd, offset, count) (0)
 #endif
 
 static ssize_t

@@ -218,7 +217,7 @@ do_pread(
 	if (!vectors)
 		return pread(fd, buffer, min(count, buffer_size), offset);
 
-	return do_preadv(fd, offset, count, buffer_size);
+	return do_preadv(fd, offset, count);
 }
 
 static int

io/pwrite.c +2 -3

index a89edfd0..f75c6164 100644
@@ -66,7 +66,6 @@ do_pwritev(
 	int		fd,
 	off64_t		offset,
 	size_t		count,
-	size_t		buffer_size,
 	int 		pwritev2_flags)
 {
 	int vecs = 0;

@@ -102,7 +101,7 @@ do_pwritev(
 	return bytes;
 }
 #else
-#define do_pwritev(fd, offset, count, buffer_size, pwritev2_flags) (0)
+#define do_pwritev(fd, offset, count, pwritev2_flags) (0)
 #endif
 
 static ssize_t

@@ -116,7 +115,7 @@ do_pwrite(
 	if (!vectors)
 		return pwrite(fd, buffer, min(count, buffer_size), offset);
 
-	return do_pwritev(fd, offset, count, buffer_size, pwritev2_flags);
+	return do_pwritev(fd, offset, count, pwritev2_flags);
 }
 
 static int

Quoted 6 lines · Zorro Lang

do_preadv and do_pwritev all have a 'buffer_size' argument, but they never used it. Instead of it, they use global 'buffersize' variable, which is initialized in alloc_buffer(). As the 'buffer_size' is useless, so remove it for clear code.

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

Hi Zorro, going through old patches and remembered that I never came back to this one, sorry.

I think that just removing it is ok. do_preadv & do_pwritev both use iov and buffersize, each is a global variable; there is no need to pass in buffer size any more than there is a need to pass in the iov itself. I'll go ahead & merge this as is.

Reviewed-by Eric Sandeen <[email protected]>

Thanks,
-Eric

Quoted 68 lines

---
 io/pread.c  | 7 +++----
 io/pwrite.c | 5 ++---
 2 files changed, 5 insertions(+), 7 deletions(-)

io/pread.c +3 -4

index 60650aa3..98e992b0 100644
@@ -176,8 +176,7 @@ static ssize_t
 do_preadv(
 	int		fd,
 	off64_t		offset,
-	size_t		count,
-	size_t		buffer_size)
+	size_t		count)
 {
 	int		vecs = 0;
 	ssize_t		oldlen = 0;

@@ -205,7 +204,7 @@ do_preadv(
 	return bytes;
 }
 #else
-#define do_preadv(fd, offset, count, buffer_size) (0)
+#define do_preadv(fd, offset, count) (0)
 #endif
 
 static ssize_t

@@ -218,7 +217,7 @@ do_pread(
 	if (!vectors)
 		return pread(fd, buffer, min(count, buffer_size), offset);
 
-	return do_preadv(fd, offset, count, buffer_size);
+	return do_preadv(fd, offset, count);
 }
 
 static int

io/pwrite.c +2 -3

index a89edfd0..f75c6164 100644
@@ -66,7 +66,6 @@ do_pwritev(
 	int		fd,
 	off64_t		offset,
 	size_t		count,
-	size_t		buffer_size,
 	int 		pwritev2_flags)
 {
 	int vecs = 0;

@@ -102,7 +101,7 @@ do_pwritev(
 	return bytes;
 }
 #else
-#define do_pwritev(fd, offset, count, buffer_size, pwritev2_flags) (0)
+#define do_pwritev(fd, offset, count, pwritev2_flags) (0)
 #endif
 
 static ssize_t

@@ -116,7 +115,7 @@ do_pwrite(
 	if (!vectors)
 		return pwrite(fd, buffer, min(count, buffer_size), offset);
 
-	return do_pwritev(fd, offset, count, buffer_size, pwritev2_flags);
+	return do_pwritev(fd, offset, count, pwritev2_flags);
 }
 
 static int

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.

By the chance, correct all 'ssize_t' type problems in pwrite/pread related functions.

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

---

Actrually, this's V3. V3 did more changes in io/pread.c, fix 'ssize_t' problem in io/pread.c (V2 only did in io/pwrite.c).

Thanks,
Zorro

 io/pread.c  | 12 ++++++------
 io/pwrite.c | 14 +++++++-------
 2 files changed, 13 insertions(+), 13 deletions(-)

io/pread.c +6 -6

index 7591276d..60650aa3 100644
@@ -172,12 +172,12 @@ dump_buffer(
 }
 
 #ifdef HAVE_PREADV
-static int
+static ssize_t
 do_preadv(
 	int		fd,
 	off64_t		offset,
-	ssize_t		count,
-	ssize_t		buffer_size)
+	size_t		count,
+	size_t		buffer_size)
 {
 	int		vecs = 0;
 	ssize_t		oldlen = 0;

@@ -208,12 +208,12 @@ do_preadv(
 #define do_preadv(fd, offset, count, buffer_size) (0)
 #endif
 
-static int
+static ssize_t
 do_pread(
 	int		fd,
 	off64_t		offset,
-	ssize_t		count,
-	ssize_t		buffer_size)
+	size_t		count,
+	size_t		buffer_size)
 {
 	if (!vectors)
 		return pread(fd, buffer, min(count, buffer_size), offset);

io/pwrite.c +7 -7

index 26f79579..3df976a8 100644
@@ -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,
 	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;