Even in that tree/version (which is from 9 years ago), packet-xml.c doesn't call the function itself. I don't see any out-of-tree commits to packet-xml.c in the history of that tree. The only file that includes packet-xml.h is packet-xmpp-utils.h (which is included by various XMPP dissectors), but they didn't even exist in the repo/snapshot you pointed to.
Looking with 'git annotate', the lines of these 3 functions were re-formatted by Bill Meier in a cleanup change in 2011, and I'm not going to dig further back than that. I think I'd best just leave those 3 functions in, and will add them to an exemption list in my script. But this does show that investigating these symbols can be time-consuming and inconclusive. Martin On Wed, Jan 27, 2021 at 1:46 PM Anders Broman <anders.bro...@ericsson.com> wrote: > Hi, > > Did some googling out of curiosity and found > https://jelmer.uk/klaus/wireshark/blob/e738b556d72d4db5d7df85969c15117dedd0d063/epan/dissectors/packet-xml.c > > Search for “xml_get_attrib" So it seems it was part of packet-xml.c at > some point so perhaps safe to remove… > > /Anders > > > > *From:* Wireshark-dev <wireshark-dev-boun...@wireshark.org> *On Behalf Of > *Martin Mathieson via Wireshark-dev > *Sent:* den 27 januari 2021 13:27 > *To:* Developer support list for Wireshark <wireshark-dev@wireshark.org> > *Cc:* Martin Mathieson <martin.r.mathie...@googlemail.com> > *Subject:* Re: [Wireshark-dev] Dissector functions and variables that > could be static > > > > Hi João, > > > > I agree that every function / variable needs to be looked at carefully, > but more so if they have WS_DLL_PUBLIC in a header file. > > > > I will reinstate the XML functions in my change. Hopefully, in other > places I will find clear comments saying that they are provided for calling > from private code or plugins, etc. > > > > The few occasions I've added to debian/libwireshark0.symbols have been to > unbreak the pipeline when linking between dissectors and GUI code. My pdcp > key-setting functions don't even appear there, but I don't build Windows > plugins. > > > > Martin > > > > On Wed, Jan 27, 2021 at 11:48 AM João Valverde via Wireshark-dev < > wireshark-dev@wireshark.org> wrote: > > Hi Martin, > > As you said some functions may only be used by third party plugins so > indiscriminately removing every exported but not used function would be a > bad policy. Even if they're not actually being used right now, who knows, > they may be part of some public API for plugins, so for use as needed. The > considerate way to remove them would be to have a deprecation period. > > Unfortunately debian symbols doesn't help here. It does not say anything > interesting not already said by the static/exported C keywords, because > functions that may be internal, for example exported from epan to the > wireshark GUI application (not intended to be part of a public API), must > also be included in debian symbols. > > So really IMO the analysis must be done case by case for each function. > And of course the risk of breaking someone elses plugin is always there, > however small, so I think it needs to proceed with some caution. > > On 27/01/21 11:06, Martin Mathieson via Wireshark-dev wrote: > > My most recent MR ( > https://gitlab.com/wireshark/wireshark/-/merge_requests/1829), has come > across some symbols that don't appear to be in used by our repo. > > > > dpkg-gensymbols: error: some symbols or patterns disappeared in the > symbols file: see diff output below > > 4934 > <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4934>dpkg-gensymbols: > warning: debian/libwireshark0/DEBIAN/symbols doesn't match completely > debian/libwireshark0.symbols > > 4935 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4935>--- > debian/libwireshark0.symbols (libwireshark0_3.5.0_amd64) > > 4936 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4936>+++ > dpkg-gensymbolsUhOwDI 2021-01-27 10:38:17.000000000 +0000 > > 4937 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4937>@@ > -2124,7 +2124,7 @@ > > 4938 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4938> > wsp_vals_pdu_type_ext@Base 1.9.1 > > 4939 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4939> > wsp_vals_status_ext@Base 1.9.1 > > 4940 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4940> > xml_escape@Base 1.9.1 > > 4941 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4941>- > xml_get_attrib@Base 1.9.1 > > 4942 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4942>- > xml_get_cdata@Base 1.9.1 > > 4943 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4943>- > xml_get_tag@Base 1.9.1 > > 4944 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4944>+#MISSING: > 3.5.0# xml_get_attrib@Base 1.9.1 > > 4945 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4945>+#MISSING: > 3.5.0# xml_get_cdata@Base 1.9.1 > > 4946 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4946>+#MISSING: > 3.5.0# xml_get_tag@Base 1.9.1 > > 4947 <https://gitlab.com/wireshark/wireshark/-/jobs/989357035#L4947> > zbee_zcl_init_cluster@Base 2.5.2 > > > > It may be possible that someone has a private dissector that uses these > xml accessor functions to try to pick out some interesting fields. > > > > I am guessing that I should have my script read > debian/libwireshark0.symbols to realise that these functions need to be > non-static, even if there it can't find any actual references? > > > > Martin > > > > > > > > On Tue, Jan 26, 2021 at 7:59 PM Martin Mathieson < > martin.r.mathie...@googlemail.com> wrote: > > I have done a bit more on this - I started picking off the ones at the end > of the (alphabetical) list - 2nd one is > https://gitlab.com/wireshark/wireshark/-/merge_requests/1817. Please feel > free if anyone feels motivated to tackle some of the earlier ones. The > script is much tidier now, and also checks for references from the ui > folder (this removed around 20 from the list), will take another pass > through it before creating an MR with it. > > > > Sometimes I see that header files are being used as a way to untangle the > order or functions, but just doing some static forward-declarations at the > top of the C module would be better if they are not shared. I am leery of > deleting functions that are not being called, but if they've been that way > for years they probably don't matter. > > > > Martin > > > > > > > > On Sun, Jan 24, 2021 at 11:06 PM Martin Mathieson < > martin.r.mathie...@googlemail.com> wrote: > > > > > > On Sun, Jan 24, 2021 at 8:27 PM Jirka Novak <j.no...@netsystem.cz> wrote: > > Hi, > > I checked the code I know: > > > epan/dissectors/packet-rtp-events.c (00000000000001a0 D> > rtp_event_type_values_ext) is not referred to so could be static? (in> > header) > It is used in UI, outside of dissectors. Therefore it should be exported. > > > > Yes, I can have the script also check for references from the object files > in ui to avoid reporting cases like this. > > > > > epan/dissectors/packet-rtp.c (00000000000006d0 T rtp_dyn_payload_remove) > > is not referred to so could be static? (in header) > > epan/dissectors/packet-rtp.c (0000000000000660 T > > rtp_dyn_payload_replace) is not referred to so could be static? (in > header) > > > > I think these 2 functions can be removed. > > > > > epan/dissectors/packet-rtps.c (0000000000000e20 D class_id_enum_names) > > is not referred to so could be static? > > > > This variable is able to become static. > > > > That two functions are not used at all. Can we remove them? > > Best regards, > > Jirka Novak > > > > ___________________________________________________________________________ > > Sent via: Wireshark-dev mailing list <wireshark-dev@wireshark.org> > <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 > <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 > >
___________________________________________________________________________ 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
