Re: [Wireshark-commits] rev 53473: /trunk/ui/ /trunk/ui/gtk/: main_menubar.c /trunk/ui/qt/: main_window_slots.cpp

[Evan Huus <eapache () gmail com>]

On Thu, Nov 21, 2013 at 8:25 AM,  <mmann78 () netscape net> wrote:
> Of the ones I needed/interested in ("eth", "ip", "ipv6", "tcp", "udp",
> "sctp") only "ip" had its proto_* variable in a header (in the dissectors
> directory).  Am I looking in the wrong place?

Oh, OK I thought TCP/UDP were already public as well, but I guess only
IP has been needed so far.

> Do you think that's something
> I should add for the other dissectors?  I know string compares are more
> expensive than integers, but I don't believe the integers are available.  I
> also don't want to establish precendence and have dissectors think they need
> the proto_* variable to be public.

I don't see any specific reason to keep the proto_ variables private,
so I have no objection to making them available. All of the protocols
involved in this case already have separate header files anyways.

I'm almost tempted to create a separate file just for the proto_
variables so that any code that needs to manipulate protocol IDs
doesn't have to include a bunch of different headers, but I think
that's overkill at this point.

> The string list to look through is rarely larger than 6 or 7 so I'm not sure
> the performance hit will be noticeable.

Ya, I was just thinking it would be simpler to be able to do a straight ==.

> -----Original Message-----
> From: Evan Huus <eapache () gmail com>
> To: wireshark-dev <wireshark-dev () wireshark org>
> Sent: Thu, Nov 21, 2013 7:27 am
> Subject: Re: [Wireshark-dev] [Wireshark-commits] rev 53473: /trunk/ui/
> /trunk/ui/gtk/: main_menubar.c /trunk/ui/qt/: main_window_slots.cpp
>
> Rather than doing string comparisons you should just be able to compare the
> value directly to the proto_* variable for the relevant protocol. Most of
> the
> necessary ones should already be I'm public headers.
>
> > On Nov 21, 2013, at 7:16 AM, mmann () wireshark org wrote:
> >
> > http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=53473
> >
> > User: mmann
> > Date: 2013/11/21 12:16 PM
> >
> > Log:
> > Walk list of protocol strings to determine which protocols are present in
> > a
> packet rather than rely on protocol specific items in packet_info in an
> effort
> to (eventually) reduce packet_info members.
> > Directory: /trunk/ui/gtk/
> >  Changes    Path              Action
> >  +69 -20    main_menubar.c    Modified
> >
> > Directory: /trunk/ui/qt/
> >  Changes    Path                     Action
> >  +40 -16    main_window_slots.cpp    Modified
> >
> >
> > ___________________________________________________________________________
> > 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