[PATCH] proc: proc_create() should return true if CONFIG_PROC_FS is not configured

[PATCH] proc: proc_create() should return true if CONFIG_PROC_FS is not configured

[Zhang Zhaolong]

proc_create() should return true if CONFIG_PROC_FS is not configured.
Otherwise if-statement like this "if (!proc_create())" would go to the false path.

Signed-off-by: Zhang Zhaolong <zhangzl2013@126.com>
---
 include/linux/proc_fs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index b97bf2e..1fc07b9 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -60,8 +60,8 @@ static inline struct proc_dir_entry *proc_mkdir_data(const char *name,
 	umode_t mode, struct proc_dir_entry *parent, void *data) { return NULL; }
 static inline struct proc_dir_entry *proc_mkdir_mode(const char *name,
 	umode_t mode, struct proc_dir_entry *parent) { return NULL; }
-#define proc_create(name, mode, parent, proc_fops) ({NULL;})
-#define proc_create_data(name, mode, parent, proc_fops, data) ({NULL;})
+#define proc_create(name, mode, parent, proc_fops) ({ 1; })
+#define proc_create_data(name, mode, parent, proc_fops, data) ({ 1; })
 
 static inline void proc_set_size(struct proc_dir_entry *de, loff_t size) {}
 static inline void proc_set_user(struct proc_dir_entry *de, kuid_t uid, kgid_t gid) {}
-- 
1.9.1

Re: [PATCH] proc: proc_create() should return true if CONFIG_PROC_FS is not configured

[Jonathon Reinhart]

On Wed, Feb 25, 2015 at 12:14 AM, Zhang Zhaolong <zhangzl2013@126.com> wrote:
>
> proc_create() should return true if CONFIG_PROC_FS is not configured.
> Otherwise if-statement like this "if (!proc_create())" would go to the false path.

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?

Re: [PATCH] proc: proc_create() should return true if CONFIG_PROC_FS is not configured

[David Howells]

Jonathon Reinhart <jonathon.reinhart@gmail.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.

David

Re: [PATCH] proc: proc_create() should return true if CONFIG_PROC_FS is not configured

[Jonathon Reinhart]

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.

Is breaking the build when the result of these dummy functions are assigned
to something what you're after? If that's the case, then why not remove them
altogether?

Re: [PATCH] proc: proc_create() should return true if CONFIG_PROC_FS is not configured

[Zhaolong Zhang]

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?