Wireshark mailing list archives
Re: Latest libnghttp2 checkin broken
From: Jeff Morriss <jeff.morriss.ws () gmail com>
Date: Fri, 01 May 2015 16:20:39 -0400
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 andthere 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,htonsFor 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,freeNo, 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.
___________________________________________________________________________ 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
Current thread:
- Re: Latest libnghttp2 checkin broken Alexis La Goutte (May 01)
- Re: Latest libnghttp2 checkin broken Joerg Mayer (May 01)
- Re: Latest libnghttp2 checkin broken Alexis La Goutte (May 01)
- Re: Latest libnghttp2 checkin broken Joerg Mayer (May 01)
- Re: Latest libnghttp2 checkin broken Jeff Morriss (May 01)
- Re: Latest libnghttp2 checkin broken Alexis La Goutte (May 02)
- RfD: How to handle libnghttp2 [Was: libnghttp2 checkin broken] Joerg Mayer (May 02)
- Re: RfD: How to handle libnghttp2 [Was: libnghttp2 checkin broken] Alexis La Goutte (May 03)
- Re: RfD: How to handle libnghttp2 [Was: libnghttp2 checkin broken] Joerg Mayer (May 03)
- Re: RfD: How to handle libnghttp2 [Was: libnghttp2 checkin broken] Alexis La Goutte (May 03)
- Re: Latest libnghttp2 checkin broken Alexis La Goutte (May 01)
- Re: Latest libnghttp2 checkin broken Joerg Mayer (May 01)