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

[Graham Bloice <graham.bloice@xxxxxxxxxxxxx>]

On 21 April 2016 at 18:31, Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> wrote:

On Thu, Apr 21, 2016 at 8:15 AM, Graham Bloice <graham.bloice@xxxxxxxxxxxxx> wrote:

The latest update to the change no longer checks .l files, so no errors are produced now, just warnings.

This leaves one last issue, the command line for the checkAPIs call in epan\dissectors is too long for a Visual Studio solution \ msbuild which unhelpfully drops a single character at (I think) 8192 byte intervals.  This leads to erroneous file names being passed into checkAPIs and output such as:

Remind me, what decade are we in? ;-)

It's a bug that I'm convinced I've come across before, I'll actually report it this time.  There is a maximum command line length (https://support.microsoft.com/en-gb/kb/830473) of allegedly 8191 bytes, but that's for cmd.exe.  This is being passed to Cygwin Perl from msbuild and I don't think cmd.exe is involved then.  The limit when programmatically creating a process using the Windows API (https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396) is 32768 bytes.

 

 

  No such file: "paket-h261.c" at C:/buildbot/builders/windows-x86-petri-dish/windows-x86-petri-dish/build/tools/checkAPIs.pl line 2034.

  No such file: "packet-siulcrypt.c" at C:/buildbot/builders/windows-x86-petri-dish/windows-x86-petri-dish/build/tools/checkAPIs.pl line 2034.

  No such file: "packet-h25.h" at C:/buildbot/builders/windows-x86-petri-dish/windows-x86-petri-dish/build/tools/checkAPIs.pl line 2034.

It's not easy in CMake to split the lists into smaller chunks (unless someone know the magic incantations) so I think the best solution is to get CMake to generate a file and modify checkAPIs.pl to take a filename parameter as the list of files to check.

There is also the issue that the check is run in the source tree, so generated files that are created in the build tree (for out-of-tree builds) are also not found by checkAPIs.pl:

  No such file: "packet-ncp2222.c" at C:/buildbot/builders/windows-x86-petri-dish/windows-x86-petri-dish/build/tools/checkAPIs.pl line 2034.

  No such file: "register.c" at C:/buildbot/builders/windows-x86-petri-dish/windows-x86-petri-dish/build/tools/checkAPIs.pl line 2034.

So, I'm looking for a Perl wrangler to add two features to checkAPIs.pl:

1. Add a parameter that takes a filename and is used as the source of files to check.
2. Add a parameter that takes a path and is used as an additional path to prefix on to a filename when attempting to open a file for parsing.

For (1) the format of the file should be specified, I'm not certain yet what's easiest to produce from CMake.

Other solutions to these issues are welcome.

Me thinks this really means that we (I?) just need to bite the bullet and give checkAPIs a (per-directory) configuration file that includes things like:

• File pattern to include (*.c, *.cpp *.h or file1.c file2.c)
• An (optional) per-file configuration with things like:
• Whether to skip the file entirely
  
• Checks to perform or skip on the file

That way the make systems don't have to worry so much about it.  (I'll listen for Graham's head exploding--if this comes to pass hopefully it won't be too much wasted effort.)

All add back a Jeff head-exploding issue, remember out-of-tree builds where generated files are in another directory to the original source, item (2) above. 

I'll try to look into this; it's also reasonably likely the short path (a file-with-a-list-of-files) will have to suffice for now.

Ack. 

Though maybe at least initially having 2 modes would make it easier: one where the config file is read and used and one where checkAPIs is called directly on a file (e.g., by the commit hook).  Last time I looked at this the though of having to deal with loading different configuration files for files on the command line (that happen to be in different directories) put me off from the whole project.

Just thinking for this for about 30 secs, is there another way?  checkAPIs seems to be a very rudimentary (not meant in any derogatory way just because it's written in Perl :_)) static code analyser.  Is there any way an actual code analyser could be used with a configuration file listing the banned API's etc.?  I guess one issue with that approach is that all the static analysers I've used are quite slow, although that's maybe because I have them turned up to 11.

--

Graham Bloice