* [KJ] Why keeping lines commented out via #if 0?
@ 2007-06-02 11:59 Andi Drebes
2007-06-02 13:15 ` Robert P. J. Day
` (13 more replies)
0 siblings, 14 replies; 15+ messages in thread
From: Andi Drebes @ 2007-06-02 11:59 UTC (permalink / raw)
To: kernel-janitors
Hi!
I recently grep'd through the kernel sources and found *a lot* of lines
that are commented out using #if 0. Is there any reason to keep them?
I think that lines that are commented out should be removed from
the mainline kernel. If developers need them in future, they can create
a local copy or branch on their machines. Or am I missing the point
about the whole #if 0 thing?
Andi
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
@ 2007-06-02 13:15 ` Robert P. J. Day
2007-06-02 14:59 ` Matthew Wilcox
` (12 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Robert P. J. Day @ 2007-06-02 13:15 UTC (permalink / raw)
To: kernel-janitors
On Sat, 2 Jun 2007, Andi Drebes wrote:
> Hi!
> I recently grep'd through the kernel sources and found *a lot* of lines
> that are commented out using #if 0. Is there any reason to keep them?
> I think that lines that are commented out should be removed from
> the mainline kernel. If developers need them in future, they can create
> a local copy or branch on their machines. Or am I missing the point
> about the whole #if 0 thing?
>
> Andi
been there, done that:
http://fsdev.net/wiki/index.php?title=If_0_or_if_1_directives
rday
--
====================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA
http://fsdev.net/wiki/index.php?title=Main_Page
====================================
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
2007-06-02 13:15 ` Robert P. J. Day
@ 2007-06-02 14:59 ` Matthew Wilcox
2007-06-02 15:29 ` Robert P. J. Day
` (11 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Matthew Wilcox @ 2007-06-02 14:59 UTC (permalink / raw)
To: kernel-janitors
On Sat, Jun 02, 2007 at 11:59:22AM +0000, Andi Drebes wrote:
> I recently grep'd through the kernel sources and found *a lot* of lines
> that are commented out using #if 0. Is there any reason to keep them?
Yes -- the people who wrote the code originally put them there. Unless
you're willing to go through each one, and figure out whether it's still
useful or not for anyone reading this section of code later, it's
better to leave it alone.
Why are comments considered useful, yet commented-out sections of code
considered worthy of deletion? The latter are potentially more useful.
Of course things can be taken too far and #if 0 / #if 1 sections can
clutter up the function and make it harder to understand. But each one
needs to be taken in context and understood, not mindlessly excised.
I think the kernel-janitors need a Hypocritic Oath. First, do no harm.
I'm a little sensitive to so-called 'trivial' changes right now, given
that Kyle just spent two days tracking down breakages on parisc from
these kinds of patches.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
2007-06-02 13:15 ` Robert P. J. Day
2007-06-02 14:59 ` Matthew Wilcox
@ 2007-06-02 15:29 ` Robert P. J. Day
2007-06-02 15:47 ` Matthew Wilcox
` (10 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Robert P. J. Day @ 2007-06-02 15:29 UTC (permalink / raw)
To: kernel-janitors
On Sat, 2 Jun 2007, Matthew Wilcox wrote:
> On Sat, Jun 02, 2007 at 11:59:22AM +0000, Andi Drebes wrote:
> > I recently grep'd through the kernel sources and found *a lot* of
> > lines that are commented out using #if 0. Is there any reason to
> > keep them?
>
> Yes -- the people who wrote the code originally put them there.
with all due respect, matthew, that's an idiotic answer.
> Unless you're willing to go through each one, and figure out whether
> it's still useful or not for anyone reading this section of code
> later, it's better to leave it alone.
it's not clear that andi was suggesting a single, sweeping deletion
operation (which *would* be a bad idea). when i read his post, *i*
interpreted it as suggesting that a gradual examination might be in
order, and stuff that was clearly worthless could be removed bit by
bit.
> Why are comments considered useful, yet commented-out sections of
> code considered worthy of deletion?
gee, matt, i dunno ... could it be that, if some code has been deleted
via "#if 0", it might actually no longer have any value? could *that*
be it?
> Of course things can be taken too far and #if 0 / #if 1 sections can
> clutter up the function and make it harder to understand.
good boy, matt ... thanks for clueing in eventually.
> But each one needs to be taken in context and understood, not
> mindlessly excised.
which is how i read andi's post.
> I think the kernel-janitors need a Hypocritic Oath. First, do no
> harm. I'm a little sensitive to so-called 'trivial' changes right
> now, given that Kyle just spent two days tracking down breakages on
> parisc from these kinds of patches.
first, it's "hippocratic". second, if patches are submitted by
janitors that cause actual breakage, then i suggest it's the fault of
the official maintainers who eventually *applied* those patches. few
janitors have the authority to *force* a patch on anyone's tree, so if
a KJ patch f**ks something up, i suggest the blame lies elsewhere.
and, finally, a lot of the work being done by janitors wouldn't be
necessary if the actual developers invested just a bit of time in
keeping their code relatively clean. instead, crappy and
slowly-rotting code is allowed to accumulate in the tree to the point
where you have (and i speak from personal experience):
- misspelled and/or long-dead CONFIG variables
- long-deceased header files
- source files that are never used as part of the build under
*any* circumstances
- and, as above, hundreds upon hundreds of "#if 0" constructs,
many of which i'm sure have been there for years and have
absolutely no value
i submit, matthew, that there wouldn't even be a *need* for a kernel
janitors project if so many developers weren't so lazy and/or sloppy.
but, sadly, they are, and the annoying part of all of this is that a
lot of those developers are quite happy to leave the scut work to the
janitors, but get all excited and put out when the occasional patches
are submitted that cause breakage.
if folks like you are so irked by broken patches, then the solution is
simple -- fix all your own code and don't accept anything from
janitors. but as long as you feel like leaving that sort of
maintenance to people who offer to be janitors, then you should be
prepared to deal with the occasional bad patch.
and, you know, saying "thank you" for all that janitorial effort every
so often probably wouldn't be out of line, either.
rday
--
====================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA
http://fsdev.net/wiki/index.php?title=Main_Page
====================================
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
` (2 preceding siblings ...)
2007-06-02 15:29 ` Robert P. J. Day
@ 2007-06-02 15:47 ` Matthew Wilcox
2007-06-02 16:48 ` Robert P. J. Day
` (9 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Matthew Wilcox @ 2007-06-02 15:47 UTC (permalink / raw)
To: kernel-janitors
On Sat, Jun 02, 2007 at 11:29:54AM -0400, Robert P. J. Day wrote:
> first, it's "hippocratic".
Thanks for failing to get the joke.
> second, if patches are submitted by
> janitors that cause actual breakage, then i suggest it's the fault of
> the official maintainers who eventually *applied* those patches. few
> janitors have the authority to *force* a patch on anyone's tree, so if
> a KJ patch f**ks something up, i suggest the blame lies elsewhere.
Gotcha. Janitors are perfect, people who actually implement features
and fix bugs are sloppy and idiots.
> i submit, matthew, that there wouldn't even be a *need* for a kernel
> janitors project if so many developers weren't so lazy and/or sloppy.
> but, sadly, they are, and the annoying part of all of this is that a
> lot of those developers are quite happy to leave the scut work to the
> janitors, but get all excited and put out when the occasional patches
> are submitted that cause breakage.
>
> if folks like you are so irked by broken patches, then the solution is
> simple -- fix all your own code and don't accept anything from
> janitors. but as long as you feel like leaving that sort of
> maintenance to people who offer to be janitors, then you should be
> prepared to deal with the occasional bad patch.
>
> and, you know, saying "thank you" for all that janitorial effort every
> so often probably wouldn't be out of line, either.
I appreciate you're just starting out, and doing janitorial patches are
a great way to do that, but you really need to either lose the attitude,
or do something to earn it.
I'll even make you an offer -- want to become the ncr53c8xx maintainer?
It's in use on a whole lot of older parisc machines, and needs a fairly
significant amount of love. There's lots of commonality with the sym2
driver, so we can cooperate and share ideas. You probably don't have
hardware, so I'll also commit to getting some hardware to you and/or
access to machines with remote console and power management.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
` (3 preceding siblings ...)
2007-06-02 15:47 ` Matthew Wilcox
@ 2007-06-02 16:48 ` Robert P. J. Day
2007-06-02 18:00 ` Andi Drebes
` (8 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Robert P. J. Day @ 2007-06-02 16:48 UTC (permalink / raw)
To: kernel-janitors
andi:
some thoughts on how much of the "#if 0" stuff can be removed.
first, in a lot of cases, there's little you can do. there will be
some code that has been commented out, with no indication as to why
that was done. if you're lucky, you might be able to recognize that
the "removed" code represents a clearly deprecated or obsolete kernel
construct, in which case you can certainly take a shot at putting
together a patch and just emailing it offline to the respective
subsystem maintainer.
i've had good luck with that -- if you're not sure about the patch,
there's no point posting it to the whole list, just send it to the
maintainer and ask for some feedback. in some cases, they'll just
add it to their local tree and you're done.
in other cases, there's obviously something weird going on and, if
you want to follow up, you might need to do some detective work. as
an example, consider this snippet from
arch/blackfin/oprofile/common.c:
...
static void op_bfin_shutdown(void)
{
#if 0
/* what is the difference between shutdown and stop? */
#endif
}
...
static void op_bfin_stop(void)
{
mutex_lock(&pfmon_lock);
if (pfmon_enabled) {
model->stop();
pfmon_enabled = 0;
}
mutex_unlock(&pfmon_lock);
}
...
i don't know -- what *is* the difference? you'd think the official
blackfin maintainer should know something like that. and if you grep
under arch/blackfin, it certainly seems like no one is terribly
interested in that routine (at least under that directory):
$ grep -rw shutdown *
mach-common/ints-priority-dc.c: .shutdown = bf561_gpio_irq_shutdown
mach-common/ints-priority-sc.c: .shutdown = bfin_gpio_irq_shutdown
oprofile/common.c: /* what is the difference between shutdown and stop? */
oprofile/common.c: ops->shutdown = op_bfin_shutdown;
however, without that particular system, it's tough to compile test
a removal. in cases like that, a short email to one of the
maintainers might clear things up.
there's no one formula for this sort of cleanup, and it's typically
compounded by the fact that developers are outrageously possessive of
their code, even if it's years old and rotting. best advice: just
start perusing and pick something that looks obvious.
rday
--
====================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA
http://fsdev.net/wiki/index.php?title=Main_Page
====================================
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
` (4 preceding siblings ...)
2007-06-02 16:48 ` Robert P. J. Day
@ 2007-06-02 18:00 ` Andi Drebes
2007-06-02 18:13 ` Arnaldo Carvalho de Melo
` (7 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Andi Drebes @ 2007-06-02 18:00 UTC (permalink / raw)
To: kernel-janitors
> some thoughts on how much of the "#if 0" stuff can be removed.
> first, in a lot of cases, there's little you can do. there will be
> some code that has been commented out, with no indication as to why
> that was done.
ok, now I'm cautious with my comments because the discussion became
quite emotional (the change of views between Matthew and you). However,
I think that if there's no indication why the code was commented out and
if even the maintainer doesn't know why, I think it is quite clear that the
affected lines should be removed, because the collective memory of the
community has forgotten why it was done. If nobody knows it, I think that
nobody will ever use this code again.
> if you're lucky, you might be able to recognize that
> the "removed" code represents a clearly deprecated or obsolete kernel
> construct, in which case you can certainly take a shot at putting
> together a patch and just emailing it offline to the respective
> subsystem maintainer.
> i've had good luck with that -- if you're not sure about the patch,
> there's no point posting it to the whole list, just send it to the
> maintainer and ask for some feedback. in some cases, they'll just
> add it to their local tree and you're done.
The next time I encounter lines commented out with #if 0 and I detect something
like that, I'll follow your advice.
> in other cases, there's obviously something weird going on and, if
> you want to follow up, you might need to do some detective work.
<snip>
> in cases like that, a short email to one of the
> maintainers might clear things up.
ok.
> there's no one formula for this sort of cleanup, and it's typically
> compounded by the fact that developers are outrageously possessive of
> their code, even if it's years old and rotting. best advice: just
> start perusing and pick something that looks obvious.
ok.
Andi
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
` (5 preceding siblings ...)
2007-06-02 18:00 ` Andi Drebes
@ 2007-06-02 18:13 ` Arnaldo Carvalho de Melo
2007-06-02 18:31 ` Andi Drebes
` (6 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2007-06-02 18:13 UTC (permalink / raw)
To: kernel-janitors
On 6/2/07, Andi Drebes <lists-receive@programmierforen.de> wrote:
> > some thoughts on how much of the "#if 0" stuff can be removed.
> > first, in a lot of cases, there's little you can do. there will be
> > some code that has been commented out, with no indication as to why
> > that was done.
> ok, now I'm cautious with my comments because the discussion became
> quite emotional (the change of views between Matthew and you). However,
> I think that if there's no indication why the code was commented out and
> if even the maintainer doesn't know why, I think it is quite clear that the
> affected lines should be removed, because the collective memory of the
> community has forgotten why it was done. If nobody knows it, I think that
> nobody will ever use this code again.
And we have source control management these days, so I think this
specific cleanup case is not that contentious.
But please add a comment when somebody remembers why the #if 0 was put there :-)
- Arnaldo
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
` (6 preceding siblings ...)
2007-06-02 18:13 ` Arnaldo Carvalho de Melo
@ 2007-06-02 18:31 ` Andi Drebes
2007-06-02 18:47 ` Thomas De Schampheleire
` (5 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Andi Drebes @ 2007-06-02 18:31 UTC (permalink / raw)
To: kernel-janitors
<snip>
> > Unless you're willing to go through each one, and figure out whether
> > it's still useful or not for anyone reading this section of code
> > later, it's better to leave it alone.
>
> it's not clear that andi was suggesting a single, sweeping deletion
> operation (which *would* be a bad idea). when i read his post, *i*
> interpreted it as suggesting that a gradual examination might be in
> order, and stuff that was clearly worthless could be removed bit by
> bit.
<snip>
To clarify that affair: No, I didn't want to write a big patch that removes
those lines. I just aked for a reason why those lines should be kept in
the tree. Even if I had not asked why, I wouldn't have written such a patch.
Instead I would have inspected these lines in detail (which is btw what I'm
going to do whenever I encounter such lines in the future).
Perhaps I picked up the wrong list. Maybe I should have asked that on
the kernel-newbie mailing list.
I'm sorry this conversation got that emotional. I didn't want to bring you
to that point (please remember: it was just a *question*).
Cheers,
Andi
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
` (7 preceding siblings ...)
2007-06-02 18:31 ` Andi Drebes
@ 2007-06-02 18:47 ` Thomas De Schampheleire
2007-06-03 8:09 ` Robert P. J. Day
` (4 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Thomas De Schampheleire @ 2007-06-02 18:47 UTC (permalink / raw)
To: kernel-janitors
2007/6/2, Andi Drebes <lists-receive@programmierforen.de>:
> <snip>
> > > Unless you're willing to go through each one, and figure out whether
> > > it's still useful or not for anyone reading this section of code
> > > later, it's better to leave it alone.
> >
> > it's not clear that andi was suggesting a single, sweeping deletion
> > operation (which *would* be a bad idea). when i read his post, *i*
> > interpreted it as suggesting that a gradual examination might be in
> > order, and stuff that was clearly worthless could be removed bit by
> > bit.
> <snip>
> To clarify that affair: No, I didn't want to write a big patch that removes
> those lines. I just aked for a reason why those lines should be kept in
> the tree. Even if I had not asked why, I wouldn't have written such a patch.
> Instead I would have inspected these lines in detail (which is btw what I'm
> going to do whenever I encounter such lines in the future).
> Perhaps I picked up the wrong list. Maybe I should have asked that on
> the kernel-newbie mailing list.
>
> I'm sorry this conversation got that emotional. I didn't want to bring you
> to that point (please remember: it was just a *question*).
Hi,
I don't think you should feel guilty about that, that was something
between Matthew and Robert.
I personally think the tone of Robert's mail was somehow unnecessarily
aggressive or attacking against Matthew, even though I tend to agree
with some of the points mentioned.
Thomas
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
` (8 preceding siblings ...)
2007-06-02 18:47 ` Thomas De Schampheleire
@ 2007-06-03 8:09 ` Robert P. J. Day
2007-06-03 9:12 ` Rene Herman
` (3 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Robert P. J. Day @ 2007-06-03 8:09 UTC (permalink / raw)
To: kernel-janitors
On Sat, 2 Jun 2007, Andi Drebes wrote:
> To clarify that affair: No, I didn't want to write a big patch that
> removes those lines. I just aked for a reason why those lines should
> be kept in the tree. Even if I had not asked why, I wouldn't have
> written such a patch. Instead I would have inspected these lines in
> detail (which is btw what I'm going to do whenever I encounter such
> lines in the future).
i doubt it's a good investment of time to *explicitly* try to remove
stuff enclosed in "#if 0" -- you're probably better off picking a
specific item or topic from the list of projects and working on that,
unless you just happen to stumble across commented out stuff that
looks like a good candidate for removal.
in addition, make sure you have a good idea of what portions of the
source tree are already slated for removal, and don't waste any time
there. for example, there's no point in doing *any* cleanup of the
entire OSS sound system, since that's going to be tossed
*entirely* someday. (but don't base that kind of decision on the
feature removal file Documentation/feature-removal-schedule.txt --
that file is virtually worthless in terms of accurate information
content.)
one strategy i've used on occasion when i'm bored is to just search
the entire tree looking for the words "deprecated" or "obsolete" and
seeing what turns up.
in any event, i should probably spend a few minutes and clean up the
project list here:
http://fsdev.net/wiki/index.php?title=Kernel_Janitor%27s_Todo_List
to remove items that look like they're basically done.
rday
--
====================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA
http://fsdev.net/wiki/index.php?title=Main_Page
====================================
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
` (9 preceding siblings ...)
2007-06-03 8:09 ` Robert P. J. Day
@ 2007-06-03 9:12 ` Rene Herman
2007-06-03 10:47 ` Andi Drebes
` (2 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Rene Herman @ 2007-06-03 9:12 UTC (permalink / raw)
To: kernel-janitors
On 06/02/2007 05:29 PM, Robert P. J. Day wrote:
> On Sat, 2 Jun 2007, Matthew Wilcox wrote:
>> Why are comments considered useful, yet commented-out sections of
>> code considered worthy of deletion?
>
> gee, matt, i dunno ... could it be that, if some code has been deleted
> via "#if 0", it might actually no longer have any value? could *that*
> be it?
No ofcourse it could not. If it didn't have any value any longer it would have
been deleted, not turned into a comment.
The value #if 0 code generally retains is documentation. Documentation on how
things were done before and/or could be done in the future, but are not done
now. It's documentation for the current developer but even more importantly, a
future developer. Any "I tried this, but this didn't work quite right yet" or
"I'd like to do it this way, but this needs something else first" or similar
information this conveys is very useful, especially over maintainer changes.
Ofcourse, noone should go overboard with it but people don't. If something has
no value anymore, it's deleted. It's only kept as a comment when it _is_ seen to
have value in some or other form and as such deleting it is not a janitorial
task but something that needs a maintainer. If the code sees active maintainance
it still being there already means it's seen to still have value and if the code
does not, it should most certainly be left alone as documentation for a possible
next person stepping in.
A suggestion that any #if 0 code should preferably have a comment describing why
it's there would ofcourse not be bad.
Rene.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
` (10 preceding siblings ...)
2007-06-03 9:12 ` Rene Herman
@ 2007-06-03 10:47 ` Andi Drebes
2007-07-02 10:43 ` Vegard Nossum
2007-07-02 11:07 ` Robert P. J. Day
13 siblings, 0 replies; 15+ messages in thread
From: Andi Drebes @ 2007-06-03 10:47 UTC (permalink / raw)
To: kernel-janitors
<snip>
> Ofcourse, noone should go overboard with it but people don't. If something has
> no value anymore, it's deleted. It's only kept as a comment when it _is_ seen to
> have value in some or other form and as such deleting it is not a janitorial
> task but something that needs a maintainer. If the code sees active maintainance
> it still being there already means it's seen to still have value and if the code
> does not, it should most certainly be left alone as documentation for a possible
> next person stepping in.
>
> A suggestion that any #if 0 code should preferably have a comment describing why
> it's there would ofcourse not be bad.
Yes, I totally agree with that. Especially for people that are new to kernel stuff (me, for example)
are quite confused about the fact that there's so much code commented out. Just a
short and descriptive line that tells people why this was commented out would be very
nice and helpful.
Andi
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
` (11 preceding siblings ...)
2007-06-03 10:47 ` Andi Drebes
@ 2007-07-02 10:43 ` Vegard Nossum
2007-07-02 11:07 ` Robert P. J. Day
13 siblings, 0 replies; 15+ messages in thread
From: Vegard Nossum @ 2007-07-02 10:43 UTC (permalink / raw)
To: kernel-janitors
On 6/3/07, Rene Herman <rene.herman@gmail.com> wrote:
> On 06/02/2007 05:29 PM, Robert P. J. Day wrote:
>
> > On Sat, 2 Jun 2007, Matthew Wilcox wrote:
>
> >> Why are comments considered useful, yet commented-out sections of
> >> code considered worthy of deletion?
> >
> > gee, matt, i dunno ... could it be that, if some code has been deleted
> > via "#if 0", it might actually no longer have any value? could *that*
> > be it?
>
> No ofcourse it could not. If it didn't have any value any longer it would have
> been deleted, not turned into a comment.
>
> The value #if 0 code generally retains is documentation. Documentation on how
> things were done before and/or could be done in the future, but are not done
> now. It's documentation for the current developer but even more importantly, a
> future developer. Any "I tried this, but this didn't work quite right yet" or
> "I'd like to do it this way, but this needs something else first" or similar
> information this conveys is very useful, especially over maintainer changes.
> A suggestion that any #if 0 code should preferably have a comment describing why
> it's there would ofcourse not be bad.
In addition, I would like to mention that documentation of these #if
0s can sometimes be found in the changelogs. For a totally random
example:
$ grep -rn "^#if 0$" kernel/
kernel/params.c:27:#if 0
kernel/params.c:722:#if 0
kernel/module.c:51:#if 0
kernel/lockdep.c:204:#if 0
Now, inspecting this can be as easy as running git annotate on the
file/lineno pair. Try for instance:
$ git annotate -l -L 722 kernel/params.c
And it shows who made the change and which commit it was. In this
case, git says ef665c1a06be719ed9a6b0ad7967137258d9457a, which can be
further inspected using git log:
$ git log -p -n 1 ef665c1a06be719ed9a6b0ad7967137258d9457a
commit ef665c1a06be719ed9a6b0ad7967137258d9457a
Author: Randy Dunlap <randy.dunlap@oracle.com>
Date: Tue Feb 13 15:19:06 2007 -0800
sysfs: fix build errors: uevent with CONFIG_SYSFS=n
Fix source files to build with CONFIG_SYSFS=n.
module_subsys is not available.
SYSFS=n, MODULES=y: T:y
SYSFS=n, MODULES=n: T:y
SYSFS=y, MODULES=y: T:y
SYSFS=y, MODULES=n: T:y
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
So the #if 0 is simply a build fix where the old code is kept for
reference. I guess the sysfs_ops was at one point defined even though
CONFIG_SYSFS was unset, but I don't know. Personally, I don't see any
point in keeping the #if 0ed code. Changing it to #if 1 wouldn't even
make a difference at this point, since the variable is unused. But it
can at least be useful to have the commit number for reference and CC
the authors of that particular commit when submitting a patch for it.
Kind regards,
Vegard
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [KJ] Why keeping lines commented out via #if 0?
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
` (12 preceding siblings ...)
2007-07-02 10:43 ` Vegard Nossum
@ 2007-07-02 11:07 ` Robert P. J. Day
13 siblings, 0 replies; 15+ messages in thread
From: Robert P. J. Day @ 2007-07-02 11:07 UTC (permalink / raw)
To: kernel-janitors
it's not the "#if 0"s that amuse me ... it's the "#if 1"s:
$ grep -r "^#if 1$" * | wc -l
226
$
some of them have some real entertainment value:
...
#if 1 /* XXX screw this, I can't get the VAC flushes working
* XXX reliably... -DaveM
*/
.globl swift_flush_cache_all, swift_flush_cache_mm
.globl swift_flush_cache_range, swift_flush_cache_page
.globl swift_flush_page_for_dma
.globl swift_flush_page_to_ram
swift_flush_cache_all:
swift_flush_cache_mm:
swift_flush_cache_range:
swift_flush_cache_page:
swift_flush_page_for_dma:
swift_flush_page_to_ram:
sethi %hi(0x2000), %o0
1: subcc %o0, 0x10, %o0
add %o0, %o0, %o1
sta %g0, [%o0] ASI_M_DATAC_TAG
bne 1b
sta %g0, [%o1] ASI_M_TXTC_TAG
retl
nop
#else
...
rday
--
====================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA
http://fsdev.net/wiki/index.php?title=Main_Page
====================================
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2007-07-02 11:07 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-06-02 11:59 [KJ] Why keeping lines commented out via #if 0? Andi Drebes
2007-06-02 13:15 ` Robert P. J. Day
2007-06-02 14:59 ` Matthew Wilcox
2007-06-02 15:29 ` Robert P. J. Day
2007-06-02 15:47 ` Matthew Wilcox
2007-06-02 16:48 ` Robert P. J. Day
2007-06-02 18:00 ` Andi Drebes
2007-06-02 18:13 ` Arnaldo Carvalho de Melo
2007-06-02 18:31 ` Andi Drebes
2007-06-02 18:47 ` Thomas De Schampheleire
2007-06-03 8:09 ` Robert P. J. Day
2007-06-03 9:12 ` Rene Herman
2007-06-03 10:47 ` Andi Drebes
2007-07-02 10:43 ` Vegard Nossum
2007-07-02 11:07 ` Robert P. J. Day
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.