Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] master 9079e3a: Cheat and try to fix the
From: Anders Broman <[email protected]>
Date: Mon, 23 Jun 2014 19:50:37 +0200


Den 23 jun 2014 19:08 skrev "Evan Huus" <[email protected]>:
>
> I have nothing against the change in general, as long as we do it by adding only the original .gperf file to git and run gperf during build to generate C file.

I don't want to push this in absurdum but generating files on all platforms might be difficult and puts extra requirement on the build environment.
That said we should have the tools to and instructions on how to generate all included files. I'm not sure that's true now.

>
>
> On Mon, Jun 23, 2014 at 1:02 PM, Anders Broman <[email protected]> wrote:
>>
>>
>> Den 23 jun 2014 18:18 skrev "Bálint Réczey" <[email protected]>:
>>
>>
>> >
>> > Hi,
>> >
>> > 2014-06-23 17:54 GMT+02:00 Evan Huus <[email protected]>:
>> > > I would *really* prefer we didn't do this.
>> > Me, too. Going this way makes maintaining Wireshark really hard. And
>> > for 1% speed increase in a very specific case
>>
>> Well even small changes adds up...
>> I'd think Sip would be a fairly common usecase...
>>
>> > I generally don't agree with the approach and in this specific IMO the
>> > gains did not justify the introduction of an automatically generated,
>> > then manually slightly modified file.
>> >
>> > I would also like to ask everyone to not rush merging changes without
>> > discussion to master. I usually wait one day or two for comments
>> > before merging a review opened by me, sometimes more if others may
>> > have very different opinion implementing the patch.
>> >
>> > Thanks,
>> > Balint
>> >
>> > >
>> > >
>> > > On Mon, Jun 23, 2014 at 11:30 AM, Wireshark code review
>> > > <[email protected]> wrote:
>> > >>
>> > >> URL:
>> > >> https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=9079e3ad1d32c594309a52ccef5936d11a93a55d
>> > >> Submitter: Anders Broman ([email protected])
>> > >> Changed: branch: master
>> > >> Repository: wireshark
>> > >>
>> > >> Commits:
>> > >>
>> > >> 9079e3a by AndersBroman ([email protected]):
>> > >>
>> > >>     Cheat and try to fix the generated file manually.
>> > >>
>> > >>     Change-Id: Iabf1821aa0ef676ac4d1d7f2983460b2e671a98a
>> > >>     Reviewed-on: https://code.wireshark.org/review/2573
>> > >>     Reviewed-by: Anders Broman <[email protected]>
>> > >>
>> > >>
>> > >> Actions performed:
>> > >>
>> > >>     from  c9a5fbe   Optimize sip_is_known_sip_header()
>> > >>     adds  9079e3a   Cheat and try to fix the generated file manually.
>> > >>
>> > >>
>> > >> Summary of changes:
>> > >>  epan/dissectors/packet-sip-hdrs.c |   30 +++++++++++++++++++++++++-----
>> > >>  1 file changed, 25 insertions(+), 5 deletions(-)
>> > >>
>> > >> ___________________________________________________________________________
>> > >> Sent via:    Wireshark-commits mailing list
>> > >> <[email protected]>
>> > >> Archives:    http://www.wireshark.org/lists/wireshark-commits
>> > >> Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits
>> > >>
>> > >> mailto:[email protected]?subject=unsubscribe
>> > >
>> > >
>> > >
>> > > ___________________________________________________________________________
>> > > Sent via:    Wireshark-dev mailing list <[email protected]>
>> > > Archives:    http://www.wireshark.org/lists/wireshark-dev
>> > > Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>> > >              mailto:[email protected]?subject=unsubscribe
>> > ___________________________________________________________________________
>> > Sent via:    Wireshark-dev mailing list <[email protected]>
>> > Archives:    http://www.wireshark.org/lists/wireshark-dev
>> > Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>> >              mailto:[email protected]?subject=unsubscribe
>>
>>
>> ___________________________________________________________________________
>> Sent via:    Wireshark-dev mailing list <[email protected]>
>> Archives:    http://www.wireshark.org/lists/wireshark-dev
>> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>>              mailto:[email protected]?subject=unsubscribe
>
>
>
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <[email protected]>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:[email protected]?subject=unsubscribe