[PATCH] fs, sysfs: Change sysfs_pathname function prototype.

[PATCH] fs, sysfs: Change sysfs_pathname function prototype.

[Rakib Mullick]

Make sysfs_pathname inline, we we're hit by the following warning:


fs/sysfs/dir.c:410: warning: ‘sysfs_pathname’ defined but not used


Signed-off-by: Rakib Mullick <rakib.mullick@gmail>
---

--- linus-36/fs/sysfs/dir.c	2010-10-21 02:30:22.000000000 +0600
+++ rakib-36/fs/sysfs/dir.c	2010-10-31 17:34:46.000000000 +0600
@@ -407,7 +407,7 @@ int __sysfs_add_one(struct sysfs_addrm_c
  *
  *	XXX: does no error checking on @path size
  */
-static char *sysfs_pathname(struct sysfs_dirent *sd, char *path)
+static inline char *sysfs_pathname(struct sysfs_dirent *sd, char *path)
 {
 	if (sd->s_parent) {
 		sysfs_pathname(sd->s_parent, path);

Re: [PATCH] fs, sysfs: Change sysfs_pathname function prototype.

[Greg KH]

On Sun, Oct 31, 2010 at 05:48:36PM +0600, Rakib Mullick wrote:
> Make sysfs_pathname inline, we we're hit by the following warning:
> 
> 
> fs/sysfs/dir.c:410: warning: ‘sysfs_pathname’ defined but not used
> 
> 
> Signed-off-by: Rakib Mullick <rakib.mullick@gmail>
> ---
> 
> --- linus-36/fs/sysfs/dir.c	2010-10-21 02:30:22.000000000 +0600
> +++ rakib-36/fs/sysfs/dir.c	2010-10-31 17:34:46.000000000 +0600
> @@ -407,7 +407,7 @@ int __sysfs_add_one(struct sysfs_addrm_c
>   *
>   *	XXX: does no error checking on @path size
>   */
> -static char *sysfs_pathname(struct sysfs_dirent *sd, char *path)
> +static inline char *sysfs_pathname(struct sysfs_dirent *sd, char *path)

If it's really not used then why would marking it inline help out any?
Shouldn't we just delete the function instead?

thanks,

greg k-h

Re: [PATCH] fs, sysfs: Change sysfs_pathname function prototype.

[Rakib Mullick]

On Wed, Nov 10, 2010 at 4:44 AM, Greg KH <greg@kroah.com> wrote:
>
> If it's really not used then why would marking it inline help out any?
> Shouldn't we just delete the function instead?
>
As far as I can see, its been called recursively and also from
sysfs_add_one. Am I missing anything?


thanks
rakib

Re: [PATCH] fs, sysfs: Change sysfs_pathname function prototype.

[Greg KH]

On Wed, Nov 10, 2010 at 10:53:37AM +0600, Rakib Mullick wrote:
> On Wed, Nov 10, 2010 at 4:44 AM, Greg KH <greg@kroah.com> wrote:
> >
> > If it's really not used then why would marking it inline help out any?
> > Shouldn't we just delete the function instead?
> >
> As far as I can see, its been called recursively and also from
> sysfs_add_one. Am I missing anything?

If so, then what is the compiler warning that is being generated that
you are trying to resolve?

totally confused,

greg k-h

Re: [PATCH] fs, sysfs: Change sysfs_pathname function prototype.

[Rakib Mullick]

On Thu, Nov 11, 2010 at 12:39 AM, Greg KH <greg@kroah.com> wrote:
> On Wed, Nov 10, 2010 at 10:53:37AM +0600, Rakib Mullick wrote:
>> On Wed, Nov 10, 2010 at 4:44 AM, Greg KH <greg@kroah.com> wrote:
>> >
>> > If it's really not used then why would marking it inline help out any?
>> > Shouldn't we just delete the function instead?
>> >
>> As far as I can see, its been called recursively and also from
>> sysfs_add_one. Am I missing anything?
>
> If so, then what is the compiler warning that is being generated that
> you are trying to resolve?
>

Yes - its really confusing. Actually, sysfs_pathname is called from
WARN(). So - when #ifndef WARN is false, then we're having that
problem and sysfs_pathname isn't used that time. So, the proposed
patch isn't the correct fix. Maybe, splitting up the WARN message and
calling sysfs_pathname is the correct one. Am I right?




> totally confused,
>
> greg k-h
>

Re: [PATCH] fs, sysfs: Change sysfs_pathname function prototype.

[Greg KH]

On Thu, Nov 11, 2010 at 12:11:34PM +0600, Rakib Mullick wrote:
> On Thu, Nov 11, 2010 at 12:39 AM, Greg KH <greg@kroah.com> wrote:
> > On Wed, Nov 10, 2010 at 10:53:37AM +0600, Rakib Mullick wrote:
> >> On Wed, Nov 10, 2010 at 4:44 AM, Greg KH <greg@kroah.com> wrote:
> >> >
> >> > If it's really not used then why would marking it inline help out any?
> >> > Shouldn't we just delete the function instead?
> >> >
> >> As far as I can see, its been called recursively and also from
> >> sysfs_add_one. Am I missing anything?
> >
> > If so, then what is the compiler warning that is being generated that
> > you are trying to resolve?
> >
> 
> Yes - its really confusing. Actually, sysfs_pathname is called from
> WARN(). So - when #ifndef WARN is false, then we're having that
> problem and sysfs_pathname isn't used that time. So, the proposed
> patch isn't the correct fix. Maybe, splitting up the WARN message and
> calling sysfs_pathname is the correct one. Am I right?

perhaps, yes, that might be correct, but as almost no one builds with
WARN not enabled, is this a real issue?

thanks,

greg k-h

Re: [PATCH] fs, sysfs: Change sysfs_pathname function prototype.

[Rakib Mullick]

On 11/11/10, Greg KH <greg@kroah.com> wrote:
>
> perhaps, yes, that might be correct, but as almost no one builds with
> WARN not enabled, is this a real issue?

I'm trying to find out the config, which causes the warning (it gets
overwritten). Not all config produces the warning. Maybe - it was a
randconfig, which generates the warning. I'll again try some
randconfig and will try to find the config which generates the
warning. If I find something I'll notice. But, I think the issue is
real.



thanks,
rakib

>
> thanks,
>
> greg k-h
>

Re: [PATCH] fs, sysfs: Change sysfs_pathname function prototype.

[Rakib Mullick]

On 11/11/10, Rakib Mullick <rakib.mullick@gmail.com> wrote:
> On 11/11/10, Greg KH <greg@kroah.com> wrote:
>>
>> perhaps, yes, that might be correct, but as almost no one builds with
>> WARN not enabled, is this a real issue?
>
> I'm trying to find out the config, which causes the warning (it gets
> overwritten). Not all config produces the warning. Maybe - it was a
> randconfig, which generates the warning. I'll again try some
> randconfig and will try to find the config which generates the
> warning. If I find something I'll notice. But, I think the issue is
> real.
>
CONFIG_EMBEDDED=y and CONFIG_BUG=n produces that warning, in that case
#ifndef WARN is defined as follows:

#define WARN(condition, format...) ({					\
	int __ret_warn_on = !!(condition);				\
	unlikely(__ret_warn_on);					\
})

from above we can see, format isn't used. sysfs_pathname is passed as
format when called from sysfs_add_one through WARN(). Since format
isn't used, that's why we're having that problem. Am I right? Should I
create a patch to split up WARN() and calling sysfs_pathname ? Or is
there any better way?


thanks,
rakib

Re: [PATCH] fs, sysfs: Change sysfs_pathname function prototype.

[Greg KH]

On Sat, Nov 13, 2010 at 07:35:10PM +0600, Rakib Mullick wrote:
> On 11/11/10, Rakib Mullick <rakib.mullick@gmail.com> wrote:
> > On 11/11/10, Greg KH <greg@kroah.com> wrote:
> >>
> >> perhaps, yes, that might be correct, but as almost no one builds with
> >> WARN not enabled, is this a real issue?
> >
> > I'm trying to find out the config, which causes the warning (it gets
> > overwritten). Not all config produces the warning. Maybe - it was a
> > randconfig, which generates the warning. I'll again try some
> > randconfig and will try to find the config which generates the
> > warning. If I find something I'll notice. But, I think the issue is
> > real.
> >
> CONFIG_EMBEDDED=y and CONFIG_BUG=n produces that warning, in that case
> #ifndef WARN is defined as follows:
> 
> #define WARN(condition, format...) ({					\
> 	int __ret_warn_on = !!(condition);				\
> 	unlikely(__ret_warn_on);					\
> })
> 
> from above we can see, format isn't used. sysfs_pathname is passed as
> format when called from sysfs_add_one through WARN(). Since format
> isn't used, that's why we're having that problem. Am I right? Should I
> create a patch to split up WARN() and calling sysfs_pathname ? Or is
> there any better way?

No, you should probably just not worry about it as the number of people
using those two configuration options are in the extreme minority so
it's really not a big issue at all.

thanks,

greg k-h