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

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

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

Attached patch removes these functions.

Thanks, applied.
--
Michael

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

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.

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