[PATCH v2] can: build proc support only if CONFIG_PROC_FS is activated

[PATCH v2] can: build proc support only if CONFIG_PROC_FS is activated

[Marc Kleine-Budde]

From: Alexander Aring <aar@pengutronix.de>

When building can subsystem with CONFIG_PROC_FS=n I detected some unused
variables warning by using proc functions. In CAN the proc handling is
nicely placed in one object file. This patch adds simple add a
dependency on CONFIG_PROC_FS for CAN's proc.o file and corresponding
static inline no-op functions.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
[mkl: provide static inline noops instead of using #ifdefs]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Hello,

picking up Alex's work. I've switched from #ifdef's to no-ops.

regards,
Marc

 net/can/Makefile |  3 ++-
 net/can/af_can.h | 11 +++++++++++
 net/can/proc.c   |  3 +--
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/net/can/Makefile b/net/can/Makefile
index cef49eb1f5c7..10936754e3f2 100644
--- a/net/can/Makefile
+++ b/net/can/Makefile
@@ -3,7 +3,8 @@
 #
 
 obj-$(CONFIG_CAN)	+= can.o
-can-y			:= af_can.o proc.o
+can-y			:= af_can.o
+can-$(CONFIG_PROC_FS)	+= proc.o
 
 obj-$(CONFIG_CAN_RAW)	+= can-raw.o
 can-raw-y		:= raw.o
diff --git a/net/can/af_can.h b/net/can/af_can.h
index fca0fe9fc45a..38a79ff20022 100644
--- a/net/can/af_can.h
+++ b/net/can/af_can.h
@@ -113,8 +113,19 @@ struct s_pstats {
 extern struct dev_rcv_lists can_rx_alldev_list;
 
 /* function prototypes for the CAN networklayer procfs (proc.c) */
+#ifdef CONFIG_PROC_FS
 void can_init_proc(void);
 void can_remove_proc(void);
+#else
+static inline void can_init_proc(void)
+{
+	pr_info("can: Can't create /proc/net/can. CONFIG_PROC_FS missing!\n");
+}
+
+static inline void can_remove_proc(void)
+{
+}
+#endif
 void can_stat_update(unsigned long data);
 
 /* structures and variables from af_can.c needed in proc.c for reading */
diff --git a/net/can/proc.c b/net/can/proc.c
index 1a19b985a868..85ef7bb0f176 100644
--- a/net/can/proc.c
+++ b/net/can/proc.c
@@ -517,8 +517,7 @@ void can_init_proc(void)
 	can_dir = proc_mkdir("can", init_net.proc_net);
 
 	if (!can_dir) {
-		printk(KERN_INFO "can: failed to create /proc/net/can . "
-		       "CONFIG_PROC_FS missing?\n");
+		pr_info("can: failed to create /proc/net/can.\n");
 		return;
 	}
 
-- 
2.8.1

Re: [PATCH v2] can: build proc support only if CONFIG_PROC_FS is activated

[Oliver Hartkopp]

On 06/17/2016 02:08 PM, Marc Kleine-Budde wrote:
> From: Alexander Aring <aar@pengutronix.de>
>
> When building can subsystem with CONFIG_PROC_FS=n I detected some unused
> variables warning by using proc functions. In CAN the proc handling is
> nicely placed in one object file. This patch adds simple add a
> dependency on CONFIG_PROC_FS for CAN's proc.o file and corresponding
> static inline no-op functions.
>
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> [mkl: provide static inline noops instead of using #ifdefs]
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

> ---
> Hello,
>
> picking up Alex's work. I've switched from #ifdef's to no-ops.
>
> regards,
> Marc
>
>   net/can/Makefile |  3 ++-
>   net/can/af_can.h | 11 +++++++++++
>   net/can/proc.c   |  3 +--
>   3 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/net/can/Makefile b/net/can/Makefile
> index cef49eb1f5c7..10936754e3f2 100644
> --- a/net/can/Makefile
> +++ b/net/can/Makefile
> @@ -3,7 +3,8 @@
>   #
>
>   obj-$(CONFIG_CAN)	+= can.o
> -can-y			:= af_can.o proc.o
> +can-y			:= af_can.o
> +can-$(CONFIG_PROC_FS)	+= proc.o
>
>   obj-$(CONFIG_CAN_RAW)	+= can-raw.o
>   can-raw-y		:= raw.o
> diff --git a/net/can/af_can.h b/net/can/af_can.h
> index fca0fe9fc45a..38a79ff20022 100644
> --- a/net/can/af_can.h
> +++ b/net/can/af_can.h
> @@ -113,8 +113,19 @@ struct s_pstats {
>   extern struct dev_rcv_lists can_rx_alldev_list;
>
>   /* function prototypes for the CAN networklayer procfs (proc.c) */
> +#ifdef CONFIG_PROC_FS
>   void can_init_proc(void);
>   void can_remove_proc(void);
> +#else
> +static inline void can_init_proc(void)
> +{
> +	pr_info("can: Can't create /proc/net/can. CONFIG_PROC_FS missing!\n");
> +}
> +
> +static inline void can_remove_proc(void)
> +{
> +}
> +#endif
>   void can_stat_update(unsigned long data);
>
>   /* structures and variables from af_can.c needed in proc.c for reading */
> diff --git a/net/can/proc.c b/net/can/proc.c
> index 1a19b985a868..85ef7bb0f176 100644
> --- a/net/can/proc.c
> +++ b/net/can/proc.c
> @@ -517,8 +517,7 @@ void can_init_proc(void)
>   	can_dir = proc_mkdir("can", init_net.proc_net);
>
>   	if (!can_dir) {
> -		printk(KERN_INFO "can: failed to create /proc/net/can . "
> -		       "CONFIG_PROC_FS missing?\n");
> +		pr_info("can: failed to create /proc/net/can.\n");
>   		return;
>   	}
>
>

Re: [PATCH v2] can: build proc support only if CONFIG_PROC_FS is activated

[Oliver Hartkopp]

Hi Alexander,

there's a procfs dependency of the CAN_BCM too.

On 06/17/2016 02:08 PM, Marc Kleine-Budde wrote:
> From: Alexander Aring <aar@pengutronix.de>
>
> When building can subsystem with CONFIG_PROC_FS=n I detected some unused
> variables warning by using proc functions.

Did you miss to switch on CAN_BCM when doing a CONFIG_PROC_FS disabled test?

Or is the procfs subsystem that intelligent that it just disables the 
functionality so that the users (e.g. in bcm.c) don't need to care about 
CONFIG_PROC_FS.

Regards,
Oliver

> In CAN the proc handling is
> nicely placed in one object file. This patch adds simple add a
> dependency on CONFIG_PROC_FS for CAN's proc.o file and corresponding
> static inline no-op functions.
>
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> [mkl: provide static inline noops instead of using #ifdefs]
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> Hello,
>
> picking up Alex's work. I've switched from #ifdef's to no-ops.
>
> regards,
> Marc
>
>   net/can/Makefile |  3 ++-
>   net/can/af_can.h | 11 +++++++++++
>   net/can/proc.c   |  3 +--
>   3 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/net/can/Makefile b/net/can/Makefile
> index cef49eb1f5c7..10936754e3f2 100644
> --- a/net/can/Makefile
> +++ b/net/can/Makefile
> @@ -3,7 +3,8 @@
>   #
>
>   obj-$(CONFIG_CAN)	+= can.o
> -can-y			:= af_can.o proc.o
> +can-y			:= af_can.o
> +can-$(CONFIG_PROC_FS)	+= proc.o
>
>   obj-$(CONFIG_CAN_RAW)	+= can-raw.o
>   can-raw-y		:= raw.o
> diff --git a/net/can/af_can.h b/net/can/af_can.h
> index fca0fe9fc45a..38a79ff20022 100644
> --- a/net/can/af_can.h
> +++ b/net/can/af_can.h
> @@ -113,8 +113,19 @@ struct s_pstats {
>   extern struct dev_rcv_lists can_rx_alldev_list;
>
>   /* function prototypes for the CAN networklayer procfs (proc.c) */
> +#ifdef CONFIG_PROC_FS
>   void can_init_proc(void);
>   void can_remove_proc(void);
> +#else
> +static inline void can_init_proc(void)
> +{
> +	pr_info("can: Can't create /proc/net/can. CONFIG_PROC_FS missing!\n");
> +}
> +
> +static inline void can_remove_proc(void)
> +{
> +}
> +#endif
>   void can_stat_update(unsigned long data);
>
>   /* structures and variables from af_can.c needed in proc.c for reading */
> diff --git a/net/can/proc.c b/net/can/proc.c
> index 1a19b985a868..85ef7bb0f176 100644
> --- a/net/can/proc.c
> +++ b/net/can/proc.c
> @@ -517,8 +517,7 @@ void can_init_proc(void)
>   	can_dir = proc_mkdir("can", init_net.proc_net);
>
>   	if (!can_dir) {
> -		printk(KERN_INFO "can: failed to create /proc/net/can . "
> -		       "CONFIG_PROC_FS missing?\n");
> +		pr_info("can: failed to create /proc/net/can.\n");
>   		return;
>   	}
>
>

Re: [PATCH v2] can: build proc support only if CONFIG_PROC_FS is activated

[Alexander Aring]

Hi,

On 06/18/2016 10:57 AM, Oliver Hartkopp wrote:
> Hi Alexander,
> 
> there's a procfs dependency of the CAN_BCM too.
> 
> On 06/17/2016 02:08 PM, Marc Kleine-Budde wrote:
>> From: Alexander Aring <aar@pengutronix.de>
>>
>> When building can subsystem with CONFIG_PROC_FS=n I detected some unused
>> variables warning by using proc functions.
> 
> Did you miss to switch on CAN_BCM when doing a CONFIG_PROC_FS disabled test?
> 

I detected the unused variable warnings while running randconfig, the
first time I didn't saw the CAN_BCM but the second time (yet another
randconfig) I saw CAN_BCM.

net/can/bcm.c:235:37: warning: ‘bcm_proc_fops’ defined but not used
[-Wunused-const-variable=]
 static const struct file_operations bcm_proc_fops = {

> Or is the procfs subsystem that intelligent that it just disables the functionality so that the users (e.g. in bcm.c) don't need to care about CONFIG_PROC_FS.
> 

It is such intelligent, but you have no static inline no-op's for
functions which returns a pointer. It's:

#define proc_fs_foobar NULL

will be used and this occurs warnings while compiling. In case of
can-proc you really don't need to link against it if CONFIG_PROC_FS
isn't set - will turn off the warnings and compiles faster.

The CAN_BCM should be fixed as well, most subsystem puts ifdefs arounds
the proc functionality if it's not well outsourced like can-proc.c.

Who will send a patch now?

- Alex

Re: [PATCH v2] can: build proc support only if CONFIG_PROC_FS is activated

[Oliver Hartkopp]

On 06/18/2016 11:36 AM, Alexander Aring wrote:
>
> Hi,
>
> On 06/18/2016 10:57 AM, Oliver Hartkopp wrote:
>> Hi Alexander,
>>
>> there's a procfs dependency of the CAN_BCM too.
>>
>> On 06/17/2016 02:08 PM, Marc Kleine-Budde wrote:
>>> From: Alexander Aring <aar@pengutronix.de>
>>>
>>> When building can subsystem with CONFIG_PROC_FS=n I detected some unused
>>> variables warning by using proc functions.
>>
>> Did you miss to switch on CAN_BCM when doing a CONFIG_PROC_FS disabled test?
>>
>
> I detected the unused variable warnings while running randconfig, the
> first time I didn't saw the CAN_BCM but the second time (yet another
> randconfig) I saw CAN_BCM.
>
> net/can/bcm.c:235:37: warning: ‘bcm_proc_fops’ defined but not used
> [-Wunused-const-variable=]
>   static const struct file_operations bcm_proc_fops = {
>
>> Or is the procfs subsystem that intelligent that it just disables the functionality so that the users (e.g. in bcm.c) don't need to care about CONFIG_PROC_FS.
>>
>
> It is such intelligent, but you have no static inline no-op's for
> functions which returns a pointer. It's:
>
> #define proc_fs_foobar NULL
>
> will be used and this occurs warnings while compiling. In case of
> can-proc you really don't need to link against it if CONFIG_PROC_FS
> isn't set - will turn off the warnings and compiles faster.
>
> The CAN_BCM should be fixed as well, most subsystem puts ifdefs arounds
> the proc functionality if it's not well outsourced like can-proc.c.
>
> Who will send a patch now?

Feel free to sent a patch!
You detected the issue and as it turns out that you are capable to 
create patches :-)

I would appreciate to see you as author then.

Thanks,
Oliver

>
> - Alex
>

Re: [PATCH v2] can: build proc support only if CONFIG_PROC_FS is activated

[Alexander Aring]

Hi,

On 06/18/2016 11:41 AM, Oliver Hartkopp wrote:
> 
> 
> On 06/18/2016 11:36 AM, Alexander Aring wrote:
>>
>> Hi,
>>
>> On 06/18/2016 10:57 AM, Oliver Hartkopp wrote:
>>> Hi Alexander,
>>>
>>> there's a procfs dependency of the CAN_BCM too.
>>>
>>> On 06/17/2016 02:08 PM, Marc Kleine-Budde wrote:
>>>> From: Alexander Aring <aar@pengutronix.de>
>>>>
>>>> When building can subsystem with CONFIG_PROC_FS=n I detected some unused
>>>> variables warning by using proc functions.
>>>
>>> Did you miss to switch on CAN_BCM when doing a CONFIG_PROC_FS disabled test?
>>>
>>
>> I detected the unused variable warnings while running randconfig, the
>> first time I didn't saw the CAN_BCM but the second time (yet another
>> randconfig) I saw CAN_BCM.
>>
>> net/can/bcm.c:235:37: warning: ‘bcm_proc_fops’ defined but not used
>> [-Wunused-const-variable=]
>>   static const struct file_operations bcm_proc_fops = {
>>
>>> Or is the procfs subsystem that intelligent that it just disables the functionality so that the users (e.g. in bcm.c) don't need to care about CONFIG_PROC_FS.
>>>
>>
>> It is such intelligent, but you have no static inline no-op's for
>> functions which returns a pointer. It's:
>>
>> #define proc_fs_foobar NULL
>>
>> will be used and this occurs warnings while compiling. In case of
>> can-proc you really don't need to link against it if CONFIG_PROC_FS
>> isn't set - will turn off the warnings and compiles faster.
>>
>> The CAN_BCM should be fixed as well, most subsystem puts ifdefs arounds
>> the proc functionality if it's not well outsourced like can-proc.c.
>>
>> Who will send a patch now?
> 
> Feel free to sent a patch!
> You detected the issue and as it turns out that you are capable to create patches :-)
> 
> I would appreciate to see you as author then.
> 

warning doesn't show anymore. I tried everything to enable the
-Wunused-const-variable=1 stuff while compiling and verbose output.

I think this was some kind of gcc change and I used my host compiler
which occured this warning in a special gcc version.

My gcc version is now and it seems they revert the change:

gcc (GCC) 6.1.1 20160602

and I don't see such warning anymore when CONFIG_PROC_FS isn't set.

Anyway I would keep the first change which makes more clear error
handling if CONFIG_PROC_FS is not build-in vs proc entry creation was
failed. At last compiling is faster.

Sorry for the noise.

- Alex

Re: [PATCH v2] can: build proc support only if CONFIG_PROC_FS is activated

[Alexander Aring]

Hi,

On 06/18/2016 01:15 PM, Alexander Aring wrote:
> 
> Hi,
> 
...
> I think this was some kind of gcc change and I used my host compiler
> which occured this warning in a special gcc version.
> 
> My gcc version is now and it seems they revert the change:
> 
> gcc (GCC) 6.1.1 20160602
> 
> and I don't see such warning anymore when CONFIG_PROC_FS isn't set.
> 

checked my mails again, it was gcc version:

gcc (GCC) 6.1.1 20160501

which produced such warnings.

- Alex