From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Miller <davem@davemloft.net>
Subject: Re: [PATCH net-next] netdevsim: remove check on return value of
 debugfs_create_dir
Date: Fri, 08 Dec 2017 10:36:56 -0500 (EST)
Message-ID: <20171208.103656.669914802592898946.davem@davemloft.net>
References: <20171208021456.3392-1-bhole_prashant_q7@lab.ntt.co.jp>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Cc: netdev@vger.kernel.org, jakub.kicinski@netronome.com
To: bhole_prashant_q7@lab.ntt.co.jp
Return-path: <netdev-owner@vger.kernel.org>
Received: from shards.monkeyblade.net ([184.105.139.130]:54720 "EHLO
        shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1753656AbdLHPg6 (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 8 Dec 2017 10:36:58 -0500
In-Reply-To: <20171208021456.3392-1-bhole_prashant_q7@lab.ntt.co.jp>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

From: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
Date: Fri,  8 Dec 2017 11:14:56 +0900

> Initial discussion started about correct handling of this condition.
> Later it was decided to remove this check altogether to make it
> consistent.
> 
> Removal of this check isn't fatal to this driver.
...
> @@ -469,8 +469,6 @@ static int __init nsim_module_init(void)
>  	int err;
>  
>  	nsim_ddir = debugfs_create_dir(DRV_NAME, NULL);
> -	if (IS_ERR(nsim_ddir))
> -		return PTR_ERR(nsim_ddir);
>  
>  	err = bus_register(&nsim_bus);

Please stop this madness.

You cannot continue if this thing returns NULL.

WHY?

Because later if you pass NULL to debugfs_create_dir() in nsim_init() do
you have any idea what it is going to do?

It's going to put the netdevsim device files into the root!

Please:

1) Handle the errors

2) Make them fatal, if DEBUGFS is enabled this should never fail except
   for memory allocation failures and we have bigger problems than
   successfully loading the netdevsim driver

3) Fix debugfs_create_dir() to have sane return value semantics so that
   people do not have to check both NULL and error pointers

Thank you.