linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Ajay Singh <ajay.kathat@microchip.com>,
	devel@driverdev.osuosl.org, venkateswara.kaja@microchip.com,
	linux-wireless@vger.kernel.org, ganesh.krishna@microchip.com,
	adham.abozaeid@microchip.com, aditya.shankar@microchip.com
Subject: Re: [PATCH] staging: wilc1000: fix undefined reference to `__this_module' compilation error
Date: Thu, 9 Aug 2018 14:13:24 +0200	[thread overview]
Message-ID: <20180809121324.GA6937@kroah.com> (raw)
In-Reply-To: <20180809104358.gg7mwmw57eawyqvd@mwanda>

On Thu, Aug 09, 2018 at 01:43:58PM +0300, Dan Carpenter wrote:
> On Thu, Aug 09, 2018 at 01:08:38PM +0300, Dan Carpenter wrote:
> > On Thu, Aug 09, 2018 at 12:13:06PM +0530, Ajay Singh wrote:
> > >  	wilc_dir = debugfs_create_dir("wilc_wifi", NULL);
> > > -	for (i = 0; i < ARRAY_SIZE(debugfs_info); i++) {
> > > -		info = &debugfs_info[i];
> > > -		debugfs_create_file(info->name,
> > > -				    info->perm,
> > > -				    wilc_dir,
> > > -				    &info->data,
> > > -				    &info->fops);
> > > +	if (IS_ERR_OR_NULL(wilc_dir)) {
> > > +		pr_err("Error creating debugfs\n");
> > > +		return -EFAULT;
> > >  	}
> > 
> > Just check for NULL.  If someone builds without debugfs enabled in their
> > .config, that's their choice.  No need to print a warning.
> > 
> 
> Reading it again, I'm not sure my email was clear...  Just do this:
> 
> 	wilc_dir = debugfs_create_dir("wilc_wifi", NULL);
> 	if (!wilc_dir)) {
> 		pr_err("Error creating debugfs/wilc_wifi/\n");
> 		return;
> 	}
> 
> If debugfs_create_dir() returns an error pointer it means all the other
> debugfs functions are also just no-op stub functions.  Passing an error
> pointer to them is harmless.

No, please never care about the return value of a debugfs call, it
should never cause your code flow to do anything different.  THis should
just be:
	wilc_dir = debugfs_create_dir("wilc_wifi", NULL);
and then keep on going.  You can always pass the return value of a
debugfs call to another one, no need to check anything.

I've done a large sweep of the kernel tree for most of this pattern for
4.18, and will keep doing it over time, as it keeps creeping back.

greg k-h

  parent reply	other threads:[~2018-08-09 14:38 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-09  6:43 [PATCH] staging: wilc1000: fix undefined reference to `__this_module' compilation error Ajay Singh
2018-08-09 10:08 ` Dan Carpenter
2018-08-09 10:43   ` Dan Carpenter
2018-08-09 11:00     ` Ajay Singh
2018-08-09 12:13     ` Greg KH [this message]
2018-08-09 12:51       ` Dan Carpenter
2018-08-09 13:18         ` Greg KH
2018-08-09 10:57   ` Ajay Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180809121324.GA6937@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=adham.abozaeid@microchip.com \
    --cc=aditya.shankar@microchip.com \
    --cc=ajay.kathat@microchip.com \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=ganesh.krishna@microchip.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=venkateswara.kaja@microchip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).