Wireshark · Wireshark-dev: Re: [Wireshark-dev] checkapi

[Jeff Morriss <jeff.morriss.ws@xxxxxxxxx>]

On Mon, Apr 11, 2016 at 12:45 PM, Graham Bloice <graham.bloice@xxxxxxxxxxxxx> wrote:

On 11 April 2016 at 16:54, Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> wrote:

On Mon, Apr 11, 2016 at 11:36 AM, Graham Bloice <graham.bloice@xxxxxxxxxxxxx> wrote:

On 11 April 2016 at 16:03, Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> wrote:

On Mon, Apr 11, 2016 at 10:29 AM, Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> wrote:

On Sun, Apr 10, 2016 at 4:44 PM, Graham Bloice <graham.bloice@xxxxxxxxxxxxx> wrote:

After creating an initial change to add checkAPI to CMake builds, following the current checks done by nmake, I got the attached (massaged) output.

 

CUSTOMBUILD : error : Found prohibited APIs in guid-utils.c: _snwprintf [C:\buildbot\builders\windows-x86-petri-dish\windows-x86-petri-dish\build\cmbuild\epan\checkAPI_epan.vcxproj]

Gerald (in r43756) said this should be StringCchPrintf().  Not sure why.

The Windows docs say the latter function is safer; change submitted.

 

CUSTOMBUILD : error : Found prohibited APIs in ftype-guid.c: strncpy [C:\buildbot\builders\windows-x86-petri-dish\windows-x86-petri-dish\build\cmbuild\epan\ftypes\checkAPI_ftypes.vcxproj]
CUSTOMBUILD : error : Found prohibited APIs in ftype-pcre.c: strcpy [C:\buildbot\builders\windows-x86-petri-dish\windows-x86-petri-dish\build\cmbuild\epan\ftypes\checkAPI_ftypes.vcxproj]
CUSTOMBUILD : error : Found prohibited APIs in ftype-string.c: strcpy [C:\buildbot\builders\windows-x86-petri-dish\windows-x86-petri-dish\build\cmbuild\epan\ftypes\checkAPI_ftypes.vcxproj]
CUSTOMBUILD : error : Found prohibited APIs in ftype-time.c: strcpy [C:\buildbot\builders\windows-x86-petri-dish\windows-x86-petri-dish\build\cmbuild\epan\ftypes\checkAPI_ftypes.vcxproj]

Looks like these have snuck in while the buildbots weren't running checkapi; checkapis run from autotools also complains.

Actually, no, those have "always" been there; neither the nmake nor autotools checkapi ran in that directory.

There's a call to the checkapi target in ftypes in the main Makefile.nmake and in epan\ftypes\Makefile.nmake there is a checkapi target that does call checkAPIs.pl on the NONGENERATED_C_FILES, which from epan\ftypes\Makefile.common lists those files found to be at fault.

The first time I looked I just did "make -C epan/ftypes checkapis" which is what made me think it was a bug.  But the code there was ancient so I looked a bit more and eventually found that it's actually commented out in the top-level Makefile.{nmake,am}:

        cd ../ftypes
##      $(MAKE) /$(MAKEFLAGS) -f Makefile.nmake checkapi

Well Duh.  I looked at that code lots when making the CMake changes and still missed it.

So should epan and epan\ftypes be in or out?  I suppose I could replicate the commenting out :-(

Well for now I guess it should be out.  Those strcpy()'s are actually safe (assuming the caller actually uses the "get the ftype's length" function to allocate the buffer length, which the caller (ftypes.c) does).  I haven't really made up my mind whether it's worth it to go add the buffer length to the "get representation" functions just to be really, really sure that we won't overflow the buffer (and make checkAPIs happy) let alone decided to find the time to do something about it.