Declared but no defined functions

Started by Masahiko Sawadaover 6 years ago7 messages

sawada.mshk@gmail.com

over 6 years ago

1 attachment(s)

Hi,

I realized that TransactionIdAbort is declared in the transam.h but
there is not its function body. As far as I found there are three
similar functions in total by the following script.

I think the following functions are mistakenly left in the header
file. So attached patch removes them.

dsa_startup()
TransactionIdAbort()
renameatt_type()

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

remove_func_declaration.patchapplication/x-patch; name=remove_func_declaration.patchDownload

diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index 6cbb0c82c7..33fd052156 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -209,7 +209,6 @@ extern PGDLLIMPORT VariableCache ShmemVariableCache;
 extern bool TransactionIdDidCommit(TransactionId transactionId);
 extern bool TransactionIdDidAbort(TransactionId transactionId);
 extern bool TransactionIdIsKnownCompleted(TransactionId transactionId);
-extern void TransactionIdAbort(TransactionId transactionId);
 extern void TransactionIdCommitTree(TransactionId xid, int nxids, TransactionId *xids);
 extern void TransactionIdAsyncCommitTree(TransactionId xid, int nxids, TransactionId *xids, XLogRecPtr lsn);
 extern void TransactionIdAbortTree(TransactionId xid, int nxids, TransactionId *xids);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index b09afa2775..ac2bfaff1e 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -60,8 +60,6 @@ extern void SetRelationHasSubclass(Oid relationId, bool relhassubclass);
 
 extern ObjectAddress renameatt(RenameStmt *stmt);
 
-extern ObjectAddress renameatt_type(RenameStmt *stmt);
-
 extern ObjectAddress RenameConstraint(RenameStmt *stmt);
 
 extern ObjectAddress RenameRelation(RenameStmt *stmt);
diff --git a/src/include/utils/dsa.h b/src/include/utils/dsa.h
index ddd3cef8c2..991b62d28c 100644
--- a/src/include/utils/dsa.h
+++ b/src/include/utils/dsa.h
@@ -99,8 +99,6 @@ typedef pg_atomic_uint64 dsa_pointer_atomic;
  */
 typedef dsm_handle dsa_handle;
 
-extern void dsa_startup(void);
-
 extern dsa_area *dsa_create(int tranche_id);
 extern dsa_area *dsa_create_in_place(void *place, size_t size,
 									 int tranche_id, dsm_segment *segment);

Tom Lane

tgl@sss.pgh.pa.us

over 6 years ago

In reply to: Masahiko Sawada (#1)

Re: Declared but no defined functions

Masahiko Sawada <sawada.mshk@gmail.com> writes:

I think the following functions are mistakenly left in the header
file. So attached patch removes them.

dsa_startup()
TransactionIdAbort()
renameatt_type()

Agreed, these are referenced nowhere. I pushed the patch.

I realized that TransactionIdAbort is declared in the transam.h but
there is not its function body. As far as I found there are three
similar functions in total by the following script.
for func in `git ls-files | egrep "\w+\.h$" | xargs cat | egrep
"extern \w+ \w+$.*$;" | sed -e "s/.* $.*$(.*);/\1(/g"`
do
if [ `git grep "$func" -- "*.c" | wc -l` -lt 1 ];then
echo $func
fi
done

FWIW, that won't catch declarations that lack "extern", nor functions
that return pointer-to-something. (Omitting "extern" is something
I consider bad style, but other people seem to be down with it.)
Might be worth another pass to look harder?

regards, tom lane

Masahiko Sawada

sawada.mshk@gmail.com

over 6 years ago

In reply to: Tom Lane (#2)

1 attachment(s)

Re: Declared but no defined functions

On Sat, Jul 6, 2019 at 7:32 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Masahiko Sawada <sawada.mshk@gmail.com> writes:

I think the following functions are mistakenly left in the header
file. So attached patch removes them.

dsa_startup()
TransactionIdAbort()
renameatt_type()

Agreed, these are referenced nowhere. I pushed the patch.

Thanks.

I realized that TransactionIdAbort is declared in the transam.h but
there is not its function body. As far as I found there are three
similar functions in total by the following script.
for func in `git ls-files | egrep "\w+\.h$" | xargs cat | egrep
"extern \w+ \w+$.*$;" | sed -e "s/.* $.*$(.*);/\1(/g"`
do
if [ `git grep "$func" -- "*.c" | wc -l` -lt 1 ];then
echo $func
fi
done

FWIW, that won't catch declarations that lack "extern", nor functions
that return pointer-to-something. (Omitting "extern" is something
I consider bad style, but other people seem to be down with it.)
Might be worth another pass to look harder?

Indeed. I've tried to search again with the following script and got
more such functions.

for func in `git ls-files | egrep "\w+\.h$" | xargs cat | egrep -v
"(^typedef)|(DECLARE)|(BKI)" | egrep "^(extern )*[\_0-9A-Za-z]+
[\_\*0-9a-zA-Z]+ ?$.+$;$" | sed -e "s/$^extern $*[\_0-9A-Za-z]\+
$[\_0-9A-Za-z\*]\+$ \{0,1\}(.*);$/\2(/g" | sed -e "s/\*//g"`
do
if [ "`git grep "$func" -- "*.c" | wc -l`" -lt 1 ];then
echo $func
fi
done

Attached patch removes these functions.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

remove_non_defined_funcs_2.patchtext/x-patch; charset=US-ASCII; name=remove_non_defined_funcs_2.patchDownload

diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
index db13507b47..fa2c6eff5a 100644
--- a/src/bin/pg_dump/pg_backup_archiver.h
+++ b/src/bin/pg_dump/pg_backup_archiver.h
@@ -426,8 +426,6 @@ typedef struct _archiveOpts
 extern TocEntry *ArchiveEntry(Archive *AHX, CatalogId catalogId,
 							  DumpId dumpId, ArchiveOpts *opts);
 
-extern void WriteTOC(ArchiveHandle *AH);
-extern void ReadTOC(ArchiveHandle *AH);
 extern void WriteHead(ArchiveHandle *AH);
 extern void ReadHead(ArchiveHandle *AH);
 extern void WriteToc(ArchiveHandle *AH);
diff --git a/src/include/bootstrap/bootstrap.h b/src/include/bootstrap/bootstrap.h
index 03706ad4f6..85705602cb 100644
--- a/src/include/bootstrap/bootstrap.h
+++ b/src/include/bootstrap/bootstrap.h
@@ -34,8 +34,6 @@ extern int	numattr;
 
 extern void AuxiliaryProcessMain(int argc, char *argv[]) pg_attribute_noreturn();
 
-extern void err_out(void);
-
 extern void closerel(char *name);
 extern void boot_openrel(char *name);
 
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index d7f33abce3..d35b4a5061 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -604,6 +604,5 @@ typedef struct ViewOptions
 extern void RelationIncrementReferenceCount(Relation rel);
 extern void RelationDecrementReferenceCount(Relation rel);
 extern bool RelationHasUnloggedIndex(Relation rel);
-extern List *RelationGetRepsetList(Relation rel);
 
 #endif							/* REL_H */
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index c0b8e3f8ce..fcf2bc2cab 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -660,7 +660,6 @@ extern int	pqWriteReady(PGconn *conn);
 /* === in fe-secure.c === */
 
 extern int	pqsecure_initialize(PGconn *);
-extern void pqsecure_destroy(void);
 extern PostgresPollingStatusType pqsecure_open_client(PGconn *);
 extern void pqsecure_close(PGconn *);
 extern ssize_t pqsecure_read(PGconn *, void *ptr, size_t len);

Michael Paquier

michael@paquier.xyz

over 6 years ago

In reply to: Masahiko Sawada (#3)

Re: Declared but no defined functions

On Sun, Jul 07, 2019 at 07:31:12AM +0800, Masahiko Sawada wrote:

Attached patch removes these functions.

Thanks, applied.
--
Michael

Masahiko Sawada

sawada.mshk@gmail.com

over 6 years ago

In reply to: Michael Paquier (#4)

Re: Declared but no defined functions

On Sun, Jul 7, 2019 at 10:04 AM Michael Paquier <michael@paquier.xyz> wrote:

On Sun, Jul 07, 2019 at 07:31:12AM +0800, Masahiko Sawada wrote:

Attached patch removes these functions.

Thanks, applied.

Thank you!

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Ashwin Agrawal

aagrawal@pivotal.io

over 6 years ago

In reply to: Masahiko Sawada (#3)

Re: Declared but no defined functions

On Sat, Jul 6, 2019 at 4:32 PM Masahiko Sawada <sawada.mshk@gmail.com>
wrote:

Indeed. I've tried to search again with the following script and got
more such functions.

for func in `git ls-files | egrep "\w+\.h$" | xargs cat | egrep -v
"(^typedef)|(DECLARE)|(BKI)" | egrep "^(extern )*[\_0-9A-Za-z]+
[\_\*0-9a-zA-Z]+ ?$.+$;$" | sed -e "s/$^extern $*[\_0-9A-Za-z]\+
$[\_0-9A-Za-z\*]\+$ \{0,1\}(.*);$/\2(/g" | sed -e "s/\*//g"`
do
if [ "`git grep "$func" -- "*.c" | wc -l`" -lt 1 ];then
echo $func
fi
done

Do we wish to make this a tool and have it in src/tools, either as part of
find_static tool after renaming that one to more generic name or
independent script.

Tom Lane

tgl@sss.pgh.pa.us

over 6 years ago

In reply to: Ashwin Agrawal (#6)

Re: Declared but no defined functions

Ashwin Agrawal <aagrawal@pivotal.io> writes:

Do we wish to make this a tool and have it in src/tools, either as part of
find_static tool after renaming that one to more generic name or
independent script.

Well, the scripts described so far are little more than jury-rigged
hacks, with lots of room for false positives *and* false negatives.
I wouldn't want to institutionalize any of them as the right way to
check for such problems. If somebody made the effort to create a
tool that was actually trustworthy, perhaps that'd be a different
story.

(Personally I was wondering whether pgindent could be hacked up to
emit things it thought were declarations of function names. I'm
not sure that I'd trust that 100% either, but at least it would have
a better shot than the grep hacks we've discussed so far. Note in
particular that pgindent would see things inside #ifdef blocks,
whether or not your local build ever sees those declarations.)

regards, tom lane