Re: #ifdef NOT_USED

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Abhijit Menon-Sen <ams(at)oryx(dot)com>
Cc: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>, systemguards(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: #ifdef NOT_USED
Date: 2005-06-28 14:17:07
Message-ID: 14879.1119968227@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Abhijit Menon-Sen <ams(at)oryx(dot)com> writes:
>> We keep such blocks of code around in case we might need to use it
>> some day.

> I think that's a bad idea. Unused code should be removed with a suitable
> CVS checkin comment (and perhaps a comment where the code was),

The code is that comment. Mostly, these blocks are subroutines that
happen not to be needed right at the moment, but form an obvious part of
a module's API and might be needed again at any time. (An example is
BufFileTellBlock in buffile.c.) If someone did need them, they'd be
unlikely to think to root through the CVS history to find if the
functionality they needed had once existed --- they'd probably waste
time rewriting the routine from scratch.

I personally think that the policy of ifdef'ing out API functions just
because they happen to be unreferenced at the moment is a bad idea;
who's to say that someone's extension module won't need the function?
But ifdef is a whole lot better than removing the code completely.

There are other common patterns for NOT_USED --- one is to document
arguments that are passed to, but currently ignored by, functions
following some API or other.

There are a few cases where a NOT_USED block represents functionality
that won't ever be resurrected --- for instance, I just recently removed
the last NOT_USED vestiges of UNDO support in xlog.c, because it's clear
now that we have no intention of going down that design path. In a
quick look, though, I did not see very many blocks that I'd favor
removing.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2005-06-28 14:24:15 Re: Occupied port warning
Previous Message Michael Fuhr 2005-06-28 14:16:06 initdb -W failure with role-capable catalogs