svn commit: trunk/busybox/networking

Robert P. J. Day rpjday at mindspring.com
Sat Jul 1 18:19:22 UTC 2006


On Sat, 1 Jul 2006, Bernhard Fischer wrote:

> On Sat, Jul 01, 2006 at 08:00:01AM -0700, rpjday at busybox.net wrote:
> >Author: rpjday
> >Date: 2006-07-01 07:59:54 -0700 (Sat, 01 Jul 2006)
> >New Revision: 15572
>
> >Modified: trunk/busybox/networking/ifupdown.c
> >===================================================================
> >--- trunk/busybox/networking/ifupdown.c	2006-07-01 14:52:12 UTC (rev 15571)
> >+++ trunk/busybox/networking/ifupdown.c	2006-07-01 14:59:54 UTC (rev 15572)
> >@@ -43,11 +43,7 @@
> > #define MAX_INTERFACE_LENGTH 10
> > #endif
> >
> >-#if 0
> >-#define debug_noise(fmt, args...) printf(fmt, ## args)
> >-#else
> > #define debug_noise(fmt, args...)
> >-#endif
> >
> > /* Forward declaration */
> > struct interface_defn_t;
>
> Whoever that may be..
> Doing stuff like this is nonsense, to say the least.

um ... no, it isn't, since the notion of removing "dead" or "unused"
code in the source tree has been discussed more than once before, and
no one seemed all that upset by the suggestion at the time.

> You're breaking the conventient possibility to turn on debugging
> facilities with what reasoning again?

because that's an immensely silly way to include conditional debugging
code.  if an individual maintainer wants to include that sort of
thing, then they should at least do it using a meaningful directive,
such as

#ifdef DEBUG

even better, each maintainer could use package-specific directives,
like

#ifdef HTTPD_DEBUG

or something like that.  there's absolutely nothing wrong with
maintainers wanting the ability to turn on debugging in their own
packages, but using "#if 0" to deal with it is just plain silly since
it requires manual editing, rather than being able to simply define
the appropriate macro at build time.

more to the point, using "#if 0" *anywhere* in the source tree is
utterly uninformative.  when you see it, it may not be clear whether
that's truly dead, obsolete code or questionable code or debugging
code or code that's there for future consideration or code that people
forgot about five years ago or what.

now, in some cases, it's clear based on a comment, as in httpd.c:

typedef enum
{
  HTTP_OK = 200,
  HTTP_MOVED_TEMPORARILY = 302,
  HTTP_BAD_REQUEST = 400,       /* malformed syntax */
  HTTP_UNAUTHORIZED = 401, /* authentication needed, respond with auth hdr */
  HTTP_NOT_FOUND = 404,
  HTTP_FORBIDDEN = 403,
  HTTP_REQUEST_TIMEOUT = 408,
  HTTP_NOT_IMPLEMENTED = 501,   /* used for unrecognized requests */
  HTTP_INTERNAL_SERVER_ERROR = 500,
#if 0 /* future use */
  HTTP_CONTINUE = 100,
  HTTP_SWITCHING_PROTOCOLS = 101,
  HTTP_CREATED = 201,
  ...

but it would *still* make more sense to use one of:

#ifdef TODO
#ifdef FUTURE_WORK
#ifdef NOT_DONE_HERE_YET

or something like that.  personally, i'd simply ban the use of "#if 0"
outright and force people to choose their directives so that they're
more meaningful.  there's simply no defense for either of "#if 0" or
"#if 1" and, one way or another, they should all be either removed or
rewritten to be more informative.

rday





More information about the busybox mailing list