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

[Graham Bloice <graham.bloice@xxxxxxxxxxxxx>]

On 11 April 2016 at 19:37, Guy Harris <guy@xxxxxxxxxxxx> wrote:

On Apr 11, 2016, at 7: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.
>>
>> While there are some warnings to be fixed up, I'm more interested in the errors as they'll make a build as bad until fixed.  Are these errors ones that should be fixed, or should the offending files be excluded from checkAPI.
>>
>> CUSTOMBUILD : error : Found prohibited APIs in C:/buildbot/builders/windows-x86-petri-dish/windows-x86-petri-dish/build/cmbuild/epan/dfilter/scanner.c: malloc,realloc,free [C:\buildbot\builders\windows-x86-petri-dish\windows-x86-petri-dish\build\cmbuild\epan\dfilter\checkAPI_dfilter.vcxproj]
>
> Historically we haven't run checkAPIs on generated code.  We're trying to keep our developers honest, not Flex et al. :-)  Taking the generated code out reduces the list of errors significantly.

Some "generated" code is "generated" by copying it from the .l/.lemon/.y/etc. files; the problem is that we'd like to check that code, but not code that's actually *generated*.

I think the original intent was to scan the input to the generation tools, but I might have messed up in the CMake conversion, not helped by CMake macros adding the generated files to the same list as the input.  I'll address this in the rework so that only input files are scanned.

 

>> CUSTOMBUILD : error : Found prohibited APIs in app_mem_usage.c: open [C:\buildbot\builders\windows-x86-petri-dish\windows-x86-petri-dish\build\cmbuild\epan\checkAPI_epan.vcxproj]
>
> The 'open()' call is Linux-only

I.e., only on Linux does that code call open().

> so technically it's not a problem (according to the comments in checkApi we prohibit 'open' because of Windows).

I.e., the issue has to do with making sure that, on Windows, a UTF-8 pathname is handled properly.  As the code that calls open() isn't used on Windows, it doesn't matter.

> Then again just replacing it with ws_open() is easy enough.

>> CUSTOMBUILD : error : Found prohibited APIs in getopt_long.c: malloc,free [C:\buildbot\builders\windows-x86-petri-dish\windows-x86-petri-dish\build\cmbuild\wsutil\checkAPI_wsutil.vcxproj]
>
> Technically this isn't our code; I'd say skipping it would be appropriate.

Yes.  It's from glibc, so, on Linux, we'll be using glibc's version of that code, and it'll be using malloc() and free() - and, on other UN*Xes that have getopt_long(), we'll be using their version of it, and it'll probably be using malloc() and free().

And the glibc version appears to "handle" malloc() failures by discarding the list of non-option flags if it can't allocate it; if that's not good enough for us, we'd have to have our own version of getopt_long(), at least on Linux, but I don't think that's worth the effort.

--

Graham Bloice