Re: direct-io: squelch maybe-uninitialized warning in do_direct_IO()

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

 



Boaz,

On Thu, 2014-07-17 at 12:37 +0300, Boaz Harrosh wrote:
> From: Paul Bolle <pebolle@xxxxxxxxxx>
> 
> The following warnings:
> 
>   fs/direct-io.c: In function ‘__blockdev_direct_IO’:
>   fs/direct-io.c:1011:12: warning: ‘to’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>   fs/direct-io.c:913:16: note: ‘to’ was declared here
>   fs/direct-io.c:1011:12: warning: ‘from’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>   fs/direct-io.c:913:10: note: ‘from’ was declared here
> 
> are false positive because dio_get_page() either fails, or sets both
> 'from' and 'to'.
> 
> Maybe it's better to move initializing "to" and "from" out of
> dio_get_page(). That _might_ make it easier for both the the reader and
> the compiler to understand what's going on. Something like this:
> 
> Christoph Hellwig said ...
> The fix of moving the code defintively looks nicer, while I think
> uninitialized_var is horrible wart that won't get anywhere near my code.
> 
> Boaz Harrosh I agree with Christoph and Paul
> 
> Signed-off-by: Paul Bolle <pebolle@xxxxxxxxxx>

This is simply not coorect!

> Signed-off-by: Boaz Harrosh <boaz@xxxxxxxxxxxxx>
> ---
>  fs/direct-io.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 98040ba..2f024fc 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -198,9 +198,8 @@ static inline int dio_refill_pages(struct dio *dio, struct dio_submit *sdio)
>   * L1 cache.
>   */
>  static inline struct page *dio_get_page(struct dio *dio,
> -		struct dio_submit *sdio, size_t *from, size_t *to)
> +		struct dio_submit *sdio)
>  {
> -	int n;
>  	if (dio_pages_present(sdio) == 0) {
>  		int ret;
>  
> @@ -209,10 +208,7 @@ static inline struct page *dio_get_page(struct dio *dio,
>  			return ERR_PTR(ret);
>  		BUG_ON(dio_pages_present(sdio) == 0);
>  	}
> -	n = sdio->head++;
> -	*from = n ? 0 : sdio->from;
> -	*to = (n == sdio->tail - 1) ? sdio->to : PAGE_SIZE;
> -	return dio->pages[n];
> +	return dio->pages[sdio->head];
>  }
>  
>  /**
> @@ -911,11 +907,15 @@ static int do_direct_IO(struct dio *dio, struct dio_submit *sdio,
>  	while (sdio->block_in_file < sdio->final_block_in_request) {
>  		struct page *page;
>  		size_t from, to;
> -		page = dio_get_page(dio, sdio, &from, &to);
> +
> +		page = dio_get_page(dio, sdio);
>  		if (IS_ERR(page)) {
>  			ret = PTR_ERR(page);
>  			goto out;
>  		}
> +		from = sdio->head ? 0 : sdio->from;
> +		to = (sdio->head == sdio->tail - 1) ? sdio->to : PAGE_SIZE;
> +		sdio->head++;
>  
>  		while (from < to) {
>  			unsigned this_chunk_bytes;	/* # of bytes mapped */


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux