Re: checkapi

[Graham Bloice <graham.bloice () trihedral com>]

On 21 April 2016 at 18:31, Jeff Morriss <jeff.morriss.ws () gmail com> wrote:

> On Thu, Apr 21, 2016 at 8:15 AM, Graham Bloice <
> graham.bloice () trihedral com> 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

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe