[For completeness of this thread] Peter took care of checkAPIs in https://code.wireshark.org/review/#/c/29754/ .
On Thu, Sep 20, 2018 at 11:03 AM Maynard, Chris <christopher.mayn...@igt.com> wrote: > I'm not sure if anyone is waiting for my feedback, but just in case ... > > I'm not against Jakub's changes. There are benefits as he mentioned, > particularly with the idea of auto-registration. The current problem as I > see it is that in its current state, the check*.pl tools won't catch > problems (such as the one I illustrated) like they used to be able to do. > > It would be great if all dissectors were converted to use the new API and > then fields were auto-registered and then all #ifndef > HAVE_HFI_SECTION_INIT ... #endif blocks could be removed. Of course that's > a large task, so in the interim, maybe it's possible for the check*.pl > tools to be updated to catch missing/duplicate/etc. entries in the hfi[] > arrays? Otherwise, the more dissectors that are written this way, the > greater the chance of errors being introduced but not being caught by the > tools. > > Thoughts? > - Chris > > -----Original Message----- > From: Wireshark-dev [mailto:wireshark-dev-boun...@wireshark.org] On > Behalf Of Michal Labedzki > Sent: Wednesday, September 19, 2018 8:39 AM > To: Alexis La Goutte <alexis.lagou...@gmail.com>; Developer support list > for Wireshark <wireshark-dev@wireshark.org> > Subject: Re: [Wireshark-dev] tools/check[hf|APIs|filtername].pl need > updating? > > I want to convert all Bluetooth dissectors to new proto tree API. Is it a > good idea? > > wt., 18 wrz 2018 o 18:23 Alexis La Goutte <alexis.lagou...@gmail.com> > napisał(a): > > > > Thanks Jakub for historic > > > > I think a good idea is revert to use "standard" API or write a tools > > for convert old dissector to new API... > > > > Cheers > > > > On Tue, Sep 18, 2018 at 6:05 PM Jakub Zawadzki < > darkjames...@darkjames.pl> wrote: > >> > >> Hi, > >> > >> W dniu 2018-09-18 16:56, Maynard, Chris napisał(a): > >> > While investigating the transum-related crash, I had suspected some > >> > unregistered hf's and ran the various tools like checkhf.pl. I > >> > then noticed that a number of dissectors seemed to have changed a > >> > bit from what I was used to before (...) > >> > >> These changes are quite old. For udp I did it in Aug 2013 > >> (88eaebaedf2e19c493ea696f633463e4f2a9a757). > >> > >> some wireshark-dev threads: > >> - https://www.wireshark.org/lists/wireshark-dev/201307/msg00222.html > >> - thread continuation: > >> https://www.wireshark.org/lists/wireshark-dev/201308/msg00035.html > >> > >> Nobody stopped me that time. > >> > >> > And I guess I missed the reasoning behind the restructuring, but > >> > what is the purpose/benefit of this restructuring > >> > >> To sum up: > >> > >> Restructuring idea was to remove usage of int hf_foo, so you would > >> need only to declare header_field_info hfi_foo (unfortunate, you > >> still need to do it on top of file). > >> > >> Benefit is no more ints, so: > >> - proto_tree_ api checks if you passed header_field_info structure, > >> - You don't need to declare int hf_foo = -1; (bonus: binary size > >> smaller 4 bytes per hf), > >> - no need for table lookup in proto_tree_add_* > >> > >> > and use of HAVE_HFI_SECTION_INIT? > >> > >> Idea was that HFI_INIT(proto_bar) would put all protocol hfi's into > >> single binary section. This way wireshark could auto-register these > >> fields without need of some indirect array (bonus: binary size > >> smaller by sizeof(void *) per hfi). > >> > >> > >> After 5 years simple grep shows that only 36 dissectors are using > >> NEW_PROTO_TREE_API, so it seems that this API is not well known or > >> not liked. > >> If it makes problem I would suggest to drop it. > >> > >> Alexis suggested the same in 2015: > >> https://www.wireshark.org/lists/wireshark-dev/201508/msg00087.html > >> > >> > >> Jakub. > > > > > > > > > > > > > > > > > > > > > > CONFIDENTIALITY NOTICE: This message is the property of International Game > Technology PLC and/or its subsidiaries and may contain proprietary, > confidential or trade secret information. This message is intended solely > for the use of the addressee. If you are not the intended recipient and > have received this message in error, please delete this message from your > system. Any unauthorized reading, distribution, copying, or other use of > this message or its attachments is strictly prohibited. > ___________________________________________________________________________ > Sent via: Wireshark-dev mailing list <wireshark-dev@wireshark.org> > Archives: https://www.wireshark.org/lists/wireshark-dev > Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev > mailto:wireshark-dev-requ...@wireshark.org > ?subject=unsubscribe
___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev@wireshark.org> Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
