Re: [PATCH v2] xfsprogs: fix wrong variable type in write_once function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux