On 21 April 2016 at 18:31, Jeff Morriss <[email protected]> wrote:

>
>
> On Thu, Apr 21, 2016 at 8:15 AM, Graham Bloice <
> [email protected]> 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 <[email protected]>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:[email protected]?subject=unsubscribe

Reply via email to