Thread: Re: [COMMITTERS] 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: Ifwe're going to expose VariableStatData for contrib
modulesto use,then we should export a reasonable set of the supporting routinestoo.
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/ > From each according to his ability. To each according to his ability to litigate.