linux-kernel - Re: [PATCH] dma-pool: no need to check return value of debugfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <936ffd36-8814-a8d2-ab94-3edc0642e5a7@arm.com>
Date:   Mon, 9 Nov 2020 11:43:25 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Tiezhu Yang <yangtiezhu@...ngson.cn>,
        Christoph Hellwig <hch@....de>,
        Marek Szyprowski <m.szyprowski@...sung.com>
Cc:     iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
        Xuefeng Li <lixuefeng@...ngson.cn>
Subject: Re: [PATCH] dma-pool: no need to check return value of debugfs_create
 functions

On 2020-11-07 10:03, Tiezhu Yang wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic should
> never do something different based on this.

Well, the only difference in behaviour is that it won't attempt to call 
further debugfs functions if they're definitely going to fail anyway, so 
no "real" logic is affected. AFAICS it's not possible for 
debugfs_create_dir() to return NULL, so this check makes no practical 
difference, just means that if it did ever fail we would save a bit of 
unnecessary work by not subsequently calling all the way down to the "if 
(IS_ERR(parent))" check in start_creating() several times.

So the given justification is a little overdramatic for this particular 
situation, when it's really just that it's not worth optimising an 
unexpected failure case, but the change itself is obviously fine.

Reviewed-by: Robin Murphy <robin.murphy@....com>

> Signed-off-by: Tiezhu Yang <yangtiezhu@...ngson.cn>
> ---
>   kernel/dma/pool.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> index d4637f7..5f84e6c 100644
> --- a/kernel/dma/pool.c
> +++ b/kernel/dma/pool.c
> @@ -38,9 +38,6 @@ static void __init dma_atomic_pool_debugfs_init(void)
>   	struct dentry *root;
>   
>   	root = debugfs_create_dir("dma_pools", NULL);
> -	if (IS_ERR_OR_NULL(root))
> -		return;
> -
>   	debugfs_create_ulong("pool_size_dma", 0400, root, &pool_size_dma);
>   	debugfs_create_ulong("pool_size_dma32", 0400, root, &pool_size_dma32);
>   	debugfs_create_ulong("pool_size_kernel", 0400, root, &pool_size_kernel);
>