Обсуждение: Declared but no defined functions

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.

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

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

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

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