Wireshark · Wireshark-dev: Re: [Wireshark-dev] Latest libnghttp2 checkin broken

[Alexis La Goutte <alexis.lagoutte@xxxxxxxxx>]

On Fri, May 1, 2015 at 10:20 PM, Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> wrote:

On 05/01/15 13:29, Joerg Mayer wrote:

Hello Alexis,

On Fri, May 01, 2015 at 03:23:37PM +0200, Alexis La Goutte wrote:

b) it can only have been committed with --no-verify as it contains
deprecated
    functions (checkAPI.pl). I hit this during conflict resolution (git
commit).

pre-commit tools is no perfect... it is for help when commit code and

there is some false postive...

Certainly the SubmittingPatches wiki page describes it that way--it's part of why I never bothered installing it.

Please verify on your system and let me know the results. Here's the
result on mine:
jmayer@egg nghttp2(master)$ ../../tools/checkAPIs.pl *
Error: Found prohibited APIs in nghttp2_helper.c: ntohl,ntohs,htonl,htons
Error: Found prohibited APIs in nghttp2_mem.c: malloc,calloc,realloc,free
Error: Found prohibited APIs in nghttp2_net.h: ntohl,ntohs,htonl,htons

For me checkAPIs is only for dissectors file
You have the same type of warning on wiretap folder :

wireshark/wiretap$ ../tools/checkAPIs.pl *
Error: Found prohibited APIs in ascend.c: malloc,free
Error: Found prohibited APIs in ascend_scanner.c: malloc,realloc,free
Error: Found prohibited APIs in k12text.c: malloc,realloc,free

No, I don't, because I do out of tree builds and all of these are generated
file ;-)

What I don't understand is, that if you have the pre-commit hook installed,
you should never have been able to commit these files.

I suppose if people do install the hook and do get the errors (and they are thought to be false positives) they just override them (as is recommended in the wiki).

I hit the problem when I did a "git commit -a" during merge conflict resolution.

We currently have one file that I know of that "validly" fails the pre-commit
script, and that is doc/packet-PROTOABBREV.c. I'm not aware of any other file
(that gets checked into the repo) that should be allowed to fail the pre-commit
hook.

The fundamental problem (at least with the checkAPIs portion of the hook) is that some files/directories have different rules than others. Currently that intelligence is built into the Makefiles which isn't really available to the hook.

A Better Way would be to build that intelligence into checkAPIs, maybe with a configuration file in each directory.

Patches are welcome :-)

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