All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zhaolong Zhang <zhaolong.zhang@windriver.com>
To: Jonathon Reinhart <jonathon.reinhart@gmail.com>,
	David Howells <dhowells@redhat.com>
Cc: Zhang Zhaolong <zhangzl2013@126.com>,
	"David S. Miller" <davem@davemloft.net>,
	Linux Netdev List <netdev@vger.kernel.org>,
	<viro@zeniv.linux.org.uk>
Subject: Re: [PATCH] proc: proc_create() should return true if CONFIG_PROC_FS is not configured
Date: Thu, 26 Feb 2015 09:21:28 +0800	[thread overview]
Message-ID: <54EE7518.2080005@windriver.com> (raw)
In-Reply-To: <CAPFHKzccoJoM9ZENAT4+zVgbw1xejHEFDrvgz57=Hidk38T+Qg@mail.gmail.com>

On 02/26/2015 05:22 AM, Jonathon Reinhart wrote:
> On Wed, Feb 25, 2015 at 6:19 AM, David Howells<dhowells@redhat.com>  wrote:
>>> >>Does that even compile? proc_create() and proc_create_data() both return
>>> >>"struct proc_dir_entry *". It doesn't make sense for those macros to "return"
>>> >>anything but NULL - certainly not 1.
>>> >>
>>> >>Besides, why shouldn't "if (!proc_create())" behave like proc_create()
>>> >>failed when
>>> >>CONFIG_PROC_FS is not enabled?  You wouldn't want the caller to start trying
>>> >>to use that ((struct proc_dir_entry *)1) would you?
>> >
>> >It's sort of arguable.  If the proc interface is merely informational and
>> >doesn't impact the function of a module to not be present, then, yes, having
>> >proc_create() return some "true" value might make sense.  It's possible to
>> >arrange things so that all the proc-related functions and data get compiled
>> >out in such a situation by not being referenced by anything.
>> >
>> >However, if the proc interface isn't merely functional, then the proc_create()
>> >failure*is*  cause for module loading failure.  But in that case, there should
>> >be a Kconfig dependency on procfs and you should never use the dummy
>> >functions.
> Of the 528 calls to proc_create(_data), only 70 are in an "if (!proc_create())"
> style conditional. Another 188 assign the result to something, presumably a
> "struct proc_dir_entry *, which are going to fail compilation with this patch.

Oh, the usage of the CONFIG_PROC_FS macro should be checked wherever 
proc_create(_data) being called, right?

      reply	other threads:[~2015-02-26  1:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-25  5:14 [PATCH] proc: proc_create() should return true if CONFIG_PROC_FS is not configured Zhang Zhaolong
2015-02-25  7:48 ` Jonathon Reinhart
2015-02-25 11:19   ` David Howells
2015-02-25 21:22     ` Jonathon Reinhart
2015-02-26  1:21       ` Zhaolong Zhang [this message]

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=54EE7518.2080005@windriver.com \
    --to=zhaolong.zhang@windriver.com \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=jonathon.reinhart@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=zhangzl2013@126.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.