Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1643679 > unrolled thread

[fs-btrfs] question about apparent useless function call

Back to article view | Back to linux.kernel

Contents

  [fs-btrfs] question about apparent useless function call "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-05-18 00:40 +0200
    Re: [fs-btrfs] question about apparent useless function call David Sterba <dsterba@suse.cz> - 2017-05-18 16:10 +0200

#1643679 — [fs-btrfs] question about apparent useless function call

Hello everybody,

While looking into Coverity ID 1398149 I ran into the following piece  
of code at fs/btrfs/zlib.c:295:

295done:
296        zlib_inflateEnd(&workspace->strm);
297        if (data_in)
298                kunmap(pages_in[page_in_index]);
299        if (!ret)
300                zero_fill_bio(orig_bio);
301        return ret;
302}

The issue here is that calling function zlib_inflateEnd() at line 296  
_seems_ to be useless once this function _seems_ to be only useful for  
its return value, which is ignored. The same for line 388.

Can someone help me understand what is the purpose of these calls?
Should them be removed, or the return value actually evaluated?

I'd really appreciate any comment on this.

Thank you!
--
Gustavo A. R. Silva

[toc] | [next] | [standalone]

#1644581

On Wed, May 17, 2017 at 05:37:44PM -0500, Gustavo A. R. Silva wrote:
> While looking into Coverity ID 1398149 I ran into the following piece  
> of code at fs/btrfs/zlib.c:295:
> 
> 295done:
> 296        zlib_inflateEnd(&workspace->strm);
> 297        if (data_in)
> 298                kunmap(pages_in[page_in_index]);
> 299        if (!ret)
> 300                zero_fill_bio(orig_bio);
> 301        return ret;
> 302}
> 
> The issue here is that calling function zlib_inflateEnd() at line 296  
> _seems_ to be useless once this function _seems_ to be only useful for  
> its return value, which is ignored. The same for line 388.
> 
> Can someone help me understand what is the purpose of these calls?
> Should them be removed, or the return value actually evaluated?

I don't think that removing the functions is a good idea, even if
they're trivial in the end. The error value needs to be checked but
seems that the only thing we can do at that point is to inform the user
that something bad happen inside the decompressor.  The cleanup (kunmap
or memset) has to be done anyway.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web