pgsql: Have find_static skip main() functions.

momjian@postgresql.org (Bruce Momjian) writes:

Have find_static skip main() functions.

Uh-oh, don't tell me you are cranking up to run *that* thing again.

This time around, please do not remove API functions just because you
can't find a reference to them in the core code. I would like to see
a posted, discussed patch first.

regards, tom lane

Tom Lane wrote:

momjian@postgresql.org (Bruce Momjian) writes:

Have find_static skip main() functions.

Uh-oh, don't tell me you are cranking up to run *that* thing again.

This time around, please do not remove API functions just because you
can't find a reference to them in the core code. I would like to see
a posted, discussed patch first.

OK, here is my match to mark items as static or not used:

ftp://momjian.us/pub/postgresql/mypatches/static

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

This time around, please do not remove API functions just because you
can't find a reference to them in the core code. I would like to see
a posted, discussed patch first.

OK, here is my match to mark items as static or not used:
ftp://momjian.us/pub/postgresql/mypatches/static

By and large, this just demonstrates the silliness of using an automated
tool for this purpose :-(. The hits in gist and gin might be valid ---
Teodor would need to comment on that --- but almost every one of the
others is a "no, don't do that". As an example, you've successfully
reverted this recent patch in toto:

2006-04-26 20:46 tgl

* src/: backend/utils/adt/selfuncs.c, include/utils/selfuncs.h: If
we're going to expose VariableStatData for contrib modules to use,
then we should export a reasonable set of the supporting routines
too.

The fundamental problem with find_static is that it hasn't got a clue
about likely future changes, nor about what we think external add-ons
might want ...

regards, tom lane

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

This time around, please do not remove API functions just because you
can't find a reference to them in the core code. I would like to see
a posted, discussed patch first.

OK, here is my match to mark items as static or not used:
ftp://momjian.us/pub/postgresql/mypatches/static

By and large, this just demonstrates the silliness of using an automated
tool for this purpose :-(. The hits in gist and gin might be valid ---
Teodor would need to comment on that --- but almost every one of the
others is a "no, don't do that". As an example, you've successfully
reverted this recent patch in toto:

2006-04-26 20:46 tgl

* src/: backend/utils/adt/selfuncs.c, include/utils/selfuncs.h: If
we're going to expose VariableStatData for contrib modules to use,
then we should export a reasonable set of the supporting routines
too.

The fundamental problem with find_static is that it hasn't got a clue
about likely future changes, nor about what we think external add-ons
might want ...

OK, I don't really have a clue either. Is any of it valid?

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

The fundamental problem with find_static is that it hasn't got a clue
about likely future changes, nor about what we think external add-ons
might want ...

OK, I don't really have a clue either. Is any of it valid?

I don't object to static-izing AlterOpClassOwner_oid or
RenameRewriteRule, and I defer to Teodor about the gist and gin
functions. The others range somewhere between "no" and "hell no".

regards, tom lane

On Sat, 2006-07-15 at 00:05 -0400, Tom Lane wrote:

The fundamental problem with find_static is that it hasn't got a clue
about likely future changes, nor about what we think external add-ons
might want

We could annotate the source to indicate that some functions are
deliberately intended to be externally visible, but not referenced
within the source tree, and then teach find_static to grok that
annotation.

-Neil

RenameRewriteRule, and I defer to Teodor about the gist and gin
functions. The others range somewhere between "no" and "hell no".

I think that gistcentryinit() and extractEntriesS() should not be a
static.

Neil Conway wrote:

On Sat, 2006-07-15 at 00:05 -0400, Tom Lane wrote:

The fundamental problem with find_static is that it hasn't got a clue
about likely future changes, nor about what we think external add-ons
might want

We could annotate the source to indicate that some functions are
deliberately intended to be externally visible, but not referenced
within the source tree, and then teach find_static to grok that
annotation.

I thought of that, but what if one gets missed? Is the tool worth the
hassle?

cheers

andrew

On Sat, Jul 15, 2006 at 09:34:57AM -0400, Andrew Dunstan wrote:

Neil Conway wrote:

We could annotate the source to indicate that some functions are
deliberately intended to be externally visible, but not referenced
within the source tree, and then teach find_static to grok that
annotation.

I thought of that, but what if one gets missed? Is the tool worth the
hassle?

The tool is just a tool. The annotation is so that some human won't
come to the conclusion that it can be removed. Teaching the tool to
skip it is just a bonus.

Some places mark external functions with DLLEXPORT but I guess we could
invent a comment that would be machine readable.

Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/