[2.6 patch] mfd/sm501.c: #if 0 unused functions

[2.6 patch] mfd/sm501.c: #if 0 unused functions

[Adrian Bunk]

This patch #if 0's the following unused functions:
- sm501_find_clock()
- sm501_gpio_get()
- sm501_gpio_set()

Signed-off-by: Adrian Bunk <bunk@kernel.org>

---

 drivers/mfd/sm501.c   |    5 +++++
 include/linux/sm501.h |   21 ---------------------
 2 files changed, 5 insertions(+), 21 deletions(-)

24190c50efec1862c20bba55a31677879fae1de3 diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
index 13bac53..54de0ff 100644
--- a/drivers/mfd/sm501.c
+++ b/drivers/mfd/sm501.c
@@ -275,6 +275,8 @@ unsigned long sm501_modify_reg(struct device *dev,
 
 EXPORT_SYMBOL_GPL(sm501_modify_reg);
 
+#if 0
+
 unsigned long sm501_gpio_get(struct device *dev,
 			     unsigned long gpio)
 {
@@ -326,6 +328,7 @@ void sm501_gpio_set(struct device *dev,
 
 EXPORT_SYMBOL_GPL(sm501_gpio_set);
 
+#endif  /*  0  */
 
 /* sm501_unit_power
  *
@@ -657,6 +660,7 @@ unsigned long sm501_set_clock(struct device *dev,
 
 EXPORT_SYMBOL_GPL(sm501_set_clock);
 
+#if 0
 /* sm501_find_clock
  *
  * finds the closest available frequency for a given clock
@@ -699,6 +703,7 @@ unsigned long sm501_find_clock(struct device *dev,
 }
 
 EXPORT_SYMBOL_GPL(sm501_find_clock);
+#endif  /*  0  */
 
 static struct sm501_device *to_sm_device(struct platform_device *pdev)
 {
diff --git a/include/linux/sm501.h b/include/linux/sm501.h
index bca1345..f416505 100644
--- a/include/linux/sm501.h
+++ b/include/linux/sm501.h
@@ -24,9 +24,6 @@ extern int sm501_unit_power(struct device *dev,
 extern unsigned long sm501_set_clock(struct device *dev,
 				     int clksrc, unsigned long freq);
 
-extern unsigned long sm501_find_clock(struct device *dev,
-				      int clksrc, unsigned long req_freq);
-
 /* sm501_misc_control
  *
  * Modify the SM501's MISC_CONTROL register
@@ -46,24 +43,6 @@ extern unsigned long sm501_modify_reg(struct device *dev,
 				      unsigned long set,
 				      unsigned long clear);
 
-/* sm501_gpio_set
- *
- * set the state of the given GPIO line
-*/
-
-extern void sm501_gpio_set(struct device *dev,
-			   unsigned long gpio,
-			   unsigned int to,
-			   unsigned int dir);
-
-/* sm501_gpio_get
- *
- * get the state of the given GPIO line
-*/
-
-extern unsigned long sm501_gpio_get(struct device *dev,
-				    unsigned long gpio);
-
 
 /* Platform data definitions */
 

Re: [2.6 patch] mfd/sm501.c: #if 0 unused functions

[Jesper Juhl]

On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
> This patch #if 0's the following unused functions:
>  - sm501_find_clock()
>  - sm501_gpio_get()
>  - sm501_gpio_set()
>

Hi Adrian,

I know we've discussed this before, but I have to comment on this once more.

Why is it that you seem to prefer adding '#if 0' around blocks of
unused code instead of removing it outright?

Last time we discussed this your argument went something along the
lines of "it'll be used again soon, so we can reinstate it then". As I
recall I commented on that but never got a reply, so I'll try again.

My position is;
A) If the code is unused and will never be used again in the future,
we should just remove it outright.
B) If the code is currently unused but will be used soon (definition
of 'soon' left as an exercise for the reader), then we should either
a) remove the code now and reinstate it later along with the code that
uses it, or b) just leave it alone so we don't have pointless churn of
two patches, one that just comments out the code and then later
another that just uncomments it.
C) If the code is currently unused and may or may not be used in the
future, do the same as in 'B'.

We currently have over two thousand instances of code inside '#if 0'
blocks in the source tree

juhl@dragon:~/kernel/linux-2.6$ pwd
/home/juhl/kernel/linux-2.6
juhl@dragon:~/kernel/linux-2.6$ egrep -R "^ *# *if +0" . | wc -l
2168

A lot of that code is years old and should never have been commented
out, it should just have been removed. We are just leaving a lot of
junk around this way that we'll never get around to cleaning out.
Janitors will be afraid to remove it since "it might be needed soon
and we can't tell" and maintainers seem reluctant to submit cleanup
patches of their own to remove it (for reasons I won't pretend to
know), so the unused code just sits there and rots.

When I removed some unused code from floppy.c a while back, one of the
blocks I removed was within '#if 0' and had been that way for years -
it should just have been removed back then instead of being commented
out.

So why exactly is it you (and others) keep doing this?


-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [2.6 patch] mfd/sm501.c: #if 0 unused functions

[Adrian Bunk]

On Sun, Apr 13, 2008 at 08:55:21PM +0200, Jesper Juhl wrote:
> On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
> > This patch #if 0's the following unused functions:
> >  - sm501_find_clock()
> >  - sm501_gpio_get()
> >  - sm501_gpio_set()
> >
> 
> Hi Adrian,
> 
> I know we've discussed this before, but I have to comment on this once more.
> 
> Why is it that you seem to prefer adding '#if 0' around blocks of
> unused code instead of removing it outright?
>...

When I removed unused code outright some people complained that they 
plan to use it tomorrow or in the next millenium or whenever.

When I #if 0 it other people complain that I should remove it outright.

So whatever I do, there's always someone complaining.  ;-)

In this case the code looks as if it might get used at some point in the 
future.

But if a maintainer tells me to resend a patch with the code removed 
instead of #if 0'ed I'm always glad to do this.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] mfd/sm501.c: #if 0 unused functions

[Jesper Juhl]

On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
> On Sun, Apr 13, 2008 at 08:55:21PM +0200, Jesper Juhl wrote:
>  > On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
>  > > This patch #if 0's the following unused functions:
>  > >  - sm501_find_clock()
>  > >  - sm501_gpio_get()
>  > >  - sm501_gpio_set()
>  > >
>  >
>  > Hi Adrian,
>  >
>  > I know we've discussed this before, but I have to comment on this once more.
>  >
>  > Why is it that you seem to prefer adding '#if 0' around blocks of
>  > unused code instead of removing it outright?
>
> >...
>
>  When I removed unused code outright some people complained that they
>  plan to use it tomorrow or in the next millenium or whenever.
>
>  When I #if 0 it other people complain that I should remove it outright.
>
>  So whatever I do, there's always someone complaining.  ;-)
>
>  In this case the code looks as if it might get used at some point in the
>  future.
>
>  But if a maintainer tells me to resend a patch with the code removed
>  instead of #if 0'ed I'm always glad to do this.
>
But, you are completely ignoring the case of "the code is unused, but
will probably be used soon, so I'll just leave it alone and avoid the
churn".  Why? What's the point of commenting it out now and then
enabling it again in a month or two - isn't that just pointless churn?


> B) If the code is currently unused but will be used soon (definition
> of 'soon' left as an exercise for the reader), then we should either
> a) remove the code now and reinstate it later along with the code that
> uses it, or b) just leave it alone so we don't have pointless churn of
> two patches, one that just comments out the code and then later
> another that just uncomments it.


-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [2.6 patch] mfd/sm501.c: #if 0 unused functions

[Adrian Bunk]

On Sun, Apr 13, 2008 at 09:08:31PM +0200, Jesper Juhl wrote:
> On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
> > On Sun, Apr 13, 2008 at 08:55:21PM +0200, Jesper Juhl wrote:
> >  > On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
> >  > > This patch #if 0's the following unused functions:
> >  > >  - sm501_find_clock()
> >  > >  - sm501_gpio_get()
> >  > >  - sm501_gpio_set()
> >  > >
> >  >
> >  > Hi Adrian,
> >  >
> >  > I know we've discussed this before, but I have to comment on this once more.
> >  >
> >  > Why is it that you seem to prefer adding '#if 0' around blocks of
> >  > unused code instead of removing it outright?
> >
> > >...
> >
> >  When I removed unused code outright some people complained that they
> >  plan to use it tomorrow or in the next millenium or whenever.
> >
> >  When I #if 0 it other people complain that I should remove it outright.
> >
> >  So whatever I do, there's always someone complaining.  ;-)
> >
> >  In this case the code looks as if it might get used at some point in the
> >  future.
> >
> >  But if a maintainer tells me to resend a patch with the code removed
> >  instead of #if 0'ed I'm always glad to do this.
> >
> But, you are completely ignoring the case of "the code is unused, but
> will probably be used soon, so I'll just leave it alone and avoid the
> churn".  Why? What's the point of commenting it out now and then
> enabling it again in a month or two - isn't that just pointless churn?
>...

It's unused since more than one year, so chances are it won't get used 
in a month or two.

As I said, if a maintainer wants me to remove it outright I'll be glad 
to do so.

And as I said, no matter whatever I do, there's always someone 
complaining...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] mfd/sm501.c: #if 0 unused functions

[Dmitri Vorobiev]

Adrian Bunk пишет:
> On Sun, Apr 13, 2008 at 09:08:31PM +0200, Jesper Juhl wrote:
>> On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
>>> On Sun, Apr 13, 2008 at 08:55:21PM +0200, Jesper Juhl wrote:
>>>  > On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
>>>  > > This patch #if 0's the following unused functions:
>>>  > >  - sm501_find_clock()
>>>  > >  - sm501_gpio_get()
>>>  > >  - sm501_gpio_set()
>>>  > >
>>>  >
>>>  > Hi Adrian,
>>>  >
>>>  > I know we've discussed this before, but I have to comment on this once more.
>>>  >
>>>  > Why is it that you seem to prefer adding '#if 0' around blocks of
>>>  > unused code instead of removing it outright?
>>>
>>>> ...
>>>  When I removed unused code outright some people complained that they
>>>  plan to use it tomorrow or in the next millenium or whenever.
>>>
>>>  When I #if 0 it other people complain that I should remove it outright.
>>>
>>>  So whatever I do, there's always someone complaining.  ;-)
>>>
>>>  In this case the code looks as if it might get used at some point in the
>>>  future.
>>>
>>>  But if a maintainer tells me to resend a patch with the code removed
>>>  instead of #if 0'ed I'm always glad to do this.
>>>
>> But, you are completely ignoring the case of "the code is unused, but
>> will probably be used soon, so I'll just leave it alone and avoid the
>> churn".  Why? What's the point of commenting it out now and then
>> enabling it again in a month or two - isn't that just pointless churn?
>> ...
> 
> It's unused since more than one year, so chances are it won't get used 
> in a month or two.
> 
> As I said, if a maintainer wants me to remove it outright I'll be glad 
> to do so.
> 
> And as I said, no matter whatever I do, there's always someone 
> complaining...

It appears to me that if you had complaint statistics, that would have
provided a solid ground for choosing the right strategy for dead code.
Offhand, I have a feeling that the fraction of cases when the code that
has been abandoned long ago is about to be reused in near future ought
to be small.

Thanks,
Dmitri

> 
> cu
> Adrian
> 

Re: [2.6 patch] mfd/sm501.c: #if 0 unused functions

[Adrian Bunk]

On Sun, Apr 13, 2008 at 11:32:58PM +0400, Dmitri Vorobiev wrote:
> Adrian Bunk пишет:
> > On Sun, Apr 13, 2008 at 09:08:31PM +0200, Jesper Juhl wrote:
> >> On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
> >>> On Sun, Apr 13, 2008 at 08:55:21PM +0200, Jesper Juhl wrote:
> >>>  > On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
> >>>  > > This patch #if 0's the following unused functions:
> >>>  > >  - sm501_find_clock()
> >>>  > >  - sm501_gpio_get()
> >>>  > >  - sm501_gpio_set()
> >>>  > >
> >>>  >
> >>>  > Hi Adrian,
> >>>  >
> >>>  > I know we've discussed this before, but I have to comment on this once more.
> >>>  >
> >>>  > Why is it that you seem to prefer adding '#if 0' around blocks of
> >>>  > unused code instead of removing it outright?
> >>>
> >>>> ...
> >>>  When I removed unused code outright some people complained that they
> >>>  plan to use it tomorrow or in the next millenium or whenever.
> >>>
> >>>  When I #if 0 it other people complain that I should remove it outright.
> >>>
> >>>  So whatever I do, there's always someone complaining.  ;-)
> >>>
> >>>  In this case the code looks as if it might get used at some point in the
> >>>  future.
> >>>
> >>>  But if a maintainer tells me to resend a patch with the code removed
> >>>  instead of #if 0'ed I'm always glad to do this.
> >>>
> >> But, you are completely ignoring the case of "the code is unused, but
> >> will probably be used soon, so I'll just leave it alone and avoid the
> >> churn".  Why? What's the point of commenting it out now and then
> >> enabling it again in a month or two - isn't that just pointless churn?
> >> ...
> > 
> > It's unused since more than one year, so chances are it won't get used 
> > in a month or two.
> > 
> > As I said, if a maintainer wants me to remove it outright I'll be glad 
> > to do so.
> > 
> > And as I said, no matter whatever I do, there's always someone 
> > complaining...
> 
> It appears to me that if you had complaint statistics, that would have
> provided a solid ground for choosing the right strategy for dead code.

I tried both ways, and in both situations some people were complaining.

No statistics required for knowing that there will anyway be complaints 
and there's nothing I can do about it.

> Offhand, I have a feeling that the fraction of cases when the code that
> has been abandoned long ago is about to be reused in near future ought
> to be small.

I make a guess how to handle it, and the maintainer then says if he 
wants it differently.

> Thanks,
> Dmitri

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] mfd/sm501.c: #if 0 unused functions

[Jesper Juhl]

On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
> On Sun, Apr 13, 2008 at 09:08:31PM +0200, Jesper Juhl wrote:
>  > On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
>  > > On Sun, Apr 13, 2008 at 08:55:21PM +0200, Jesper Juhl wrote:
>  > >  > On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
>  > >  > > This patch #if 0's the following unused functions:
>  > >  > >  - sm501_find_clock()
>  > >  > >  - sm501_gpio_get()
>  > >  > >  - sm501_gpio_set()
>  > >  > >
>  > >  >
>  > >  > Hi Adrian,
>  > >  >
>  > >  > I know we've discussed this before, but I have to comment on this once more.
>  > >  >
>  > >  > Why is it that you seem to prefer adding '#if 0' around blocks of
>  > >  > unused code instead of removing it outright?
>  > >
>  > > >...
>  > >
>  > >  When I removed unused code outright some people complained that they
>  > >  plan to use it tomorrow or in the next millenium or whenever.
>  > >
>  > >  When I #if 0 it other people complain that I should remove it outright.
>  > >
>  > >  So whatever I do, there's always someone complaining.  ;-)
>  > >
>  > >  In this case the code looks as if it might get used at some point in the
>  > >  future.
>  > >
>  > >  But if a maintainer tells me to resend a patch with the code removed
>  > >  instead of #if 0'ed I'm always glad to do this.
>  > >
>  > But, you are completely ignoring the case of "the code is unused, but
>  > will probably be used soon, so I'll just leave it alone and avoid the
>  > churn".  Why? What's the point of commenting it out now and then
>  > enabling it again in a month or two - isn't that just pointless churn?
>
> >...
>
>  It's unused since more than one year, so chances are it won't get used
>  in a month or two.
>
Then I really don't see why you chose the '#if 0' option. In that case
it would seem to me that either "leave it alone, don't submit a patch"
or "submit a patch to remove it outright" would both be more
preferable options.

>  As I said, if a maintainer wants me to remove it outright I'll be glad
>  to do so.
>
I don't doubt that.

>  And as I said, no matter whatever I do, there's always someone
>  complaining...
>
Please don't see my comments as complaints. They are not intended as
such. I'm merely currious why we keep adding all these '#if 0's since
I don't see the point and I can just see them piling up into some huge
janitorial mountain from hell to be tackled some time in the future by
whomever is masochistic enough to try ;-)

-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [2.6 patch] mfd/sm501.c: #if 0 unused functions

[Adrian Bunk]

On Sun, Apr 13, 2008 at 09:34:24PM +0200, Jesper Juhl wrote:
>...
> Please don't see my comments as complaints. They are not intended as
> such. I'm merely currious why we keep adding all these '#if 0's since
> I don't see the point and I can just see them piling up into some huge
> janitorial mountain from hell to be tackled some time in the future by
> whomever is masochistic enough to try ;-)

The main advantage of the #if 0 solution is that it not only stops 
bloating the kernel image, but in the frequent "I might need it some day" 
case I can simply say "no problem with my patch - just remove the #if 0".

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] mfd/sm501.c: #if 0 unused functions

[Jesper Juhl]

On 13/04/2008, Adrian Bunk <bunk@kernel.org> wrote:
> On Sun, Apr 13, 2008 at 09:34:24PM +0200, Jesper Juhl wrote:
>  >...
>  > Please don't see my comments as complaints. They are not intended as
>  > such. I'm merely currious why we keep adding all these '#if 0's since
>  > I don't see the point and I can just see them piling up into some huge
>  > janitorial mountain from hell to be tackled some time in the future by
>  > whomever is masochistic enough to try ;-)
>
>
> The main advantage of the #if 0 solution is that it not only stops
>  bloating the kernel image, but in the frequent "I might need it some day"
>  case I can simply say "no problem with my patch - just remove the #if 0".
>
Heh, and in the mean time I'm tempted to submit a huge patch removing
all code within
  #if 0
  #endif
outright.
Just to get rid of all the useless code we are carrying around from
release to release.
Those 2168 '#if 0' blocks amount to quite a lot of
code^d^d^d^dcomments and most of it is just useless...  But, before I
attempt that I need to go and buy new flame proof underwear.


-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

[2.6 patch] mfd/sm501.c: #if 0 unused functions

[Adrian Bunk]

This patch #if 0's the following unused functions:
- sm501_find_clock()
- sm501_gpio_get()
- sm501_gpio_set()

Signed-off-by: Adrian Bunk <bunk@kernel.org>

---

This patch has been sent on:
- 13 Apr 2008

 drivers/mfd/sm501.c   |    5 +++++
 include/linux/sm501.h |   21 ---------------------
 2 files changed, 5 insertions(+), 21 deletions(-)

24190c50efec1862c20bba55a31677879fae1de3 diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
index 13bac53..54de0ff 100644
--- a/drivers/mfd/sm501.c
+++ b/drivers/mfd/sm501.c
@@ -275,6 +275,8 @@ unsigned long sm501_modify_reg(struct device *dev,
 
 EXPORT_SYMBOL_GPL(sm501_modify_reg);
 
+#if 0
+
 unsigned long sm501_gpio_get(struct device *dev,
 			     unsigned long gpio)
 {
@@ -326,6 +328,7 @@ void sm501_gpio_set(struct device *dev,
 
 EXPORT_SYMBOL_GPL(sm501_gpio_set);
 
+#endif  /*  0  */
 
 /* sm501_unit_power
  *
@@ -657,6 +660,7 @@ unsigned long sm501_set_clock(struct device *dev,
 
 EXPORT_SYMBOL_GPL(sm501_set_clock);
 
+#if 0
 /* sm501_find_clock
  *
  * finds the closest available frequency for a given clock
@@ -699,6 +703,7 @@ unsigned long sm501_find_clock(struct device *dev,
 }
 
 EXPORT_SYMBOL_GPL(sm501_find_clock);
+#endif  /*  0  */
 
 static struct sm501_device *to_sm_device(struct platform_device *pdev)
 {
diff --git a/include/linux/sm501.h b/include/linux/sm501.h
index bca1345..f416505 100644
--- a/include/linux/sm501.h
+++ b/include/linux/sm501.h
@@ -24,9 +24,6 @@ extern int sm501_unit_power(struct device *dev,
 extern unsigned long sm501_set_clock(struct device *dev,
 				     int clksrc, unsigned long freq);
 
-extern unsigned long sm501_find_clock(struct device *dev,
-				      int clksrc, unsigned long req_freq);
-
 /* sm501_misc_control
  *
  * Modify the SM501's MISC_CONTROL register
@@ -46,24 +43,6 @@ extern unsigned long sm501_modify_reg(struct device *dev,
 				      unsigned long set,
 				      unsigned long clear);
 
-/* sm501_gpio_set
- *
- * set the state of the given GPIO line
-*/
-
-extern void sm501_gpio_set(struct device *dev,
-			   unsigned long gpio,
-			   unsigned int to,
-			   unsigned int dir);
-
-/* sm501_gpio_get
- *
- * get the state of the given GPIO line
-*/
-
-extern unsigned long sm501_gpio_get(struct device *dev,
-				    unsigned long gpio);
-
 
 /* Platform data definitions */