Re: [Wireshark-commits] master 9079e3a: Cheat and try to fix the generated file manually.

[Evan Huus <eapache () gmail com>]

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.


On Mon, Jun 23, 2014 at 1:02 PM, Anders Broman <a.broman58 () gmail com> wrote:

> Den 23 jun 2014 18:18 skrev "Bálint Réczey" <balint () balintreczey hu>:
>
> > Hi,
> >
> > 2014-06-23 17:54 GMT+02:00 Evan Huus <eapache () gmail com>:
> > > 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
> > > <code-review-do-not-reply () wireshark org> wrote:
> > > > URL:
> https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=9079e3ad1d32c594309a52ccef5936d11a93a55d
> > > > Submitter: Anders Broman (a.broman58 () gmail com)
> > > > Changed: branch: master
> > > > Repository: wireshark
> > > >
> > > > Commits:
> > > >
> > > > 9079e3a by AndersBroman (anders.broman () ericsson com):
> > > >
> > > >     Cheat and try to fix the generated file manually.
> > > >
> > > >     Change-Id: Iabf1821aa0ef676ac4d1d7f2983460b2e671a98a
> > > >     Reviewed-on: https://code.wireshark.org/review/2573
> > > >     Reviewed-by: Anders Broman <a.broman58 () gmail com>
> > > >
> > > >
> > > > 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
> > > > <wireshark-commits () wireshark org>
> > > > Archives:    http://www.wireshark.org/lists/wireshark-commits
> > > > Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits
> > > >
> > > > mailto:wireshark-commits-request () wireshark org?subject=unsubscribe
> ___________________________________________________________________________
> > > Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
> > > Archives:    http://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:    http://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:    http://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:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe