Re: Latest libnghttp2 checkin broken

[Alexis La Goutte <alexis.lagoutte () gmail com>]

On Fri, May 1, 2015 at 10:20 PM, Jeff Morriss <jeff.morriss.ws () gmail com>
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 () 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
___________________________________________________________________________
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