ANNOUNCEMENT: Live Wireshark University & Allegro Packets online APAC Wireshark Training Session
April 17th, 2024 | 14:30-16:00 SGT (UTC+8) | Online

Wireshark-bugs: [Wireshark-bugs] [Bug 6903] Dissector for the NXP PN532 protocol

Date: Tue, 13 Mar 2012 06:44:14 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6903

--- Comment #12 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2012-03-13 06:44:14 PDT ---
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #7)
> > > > Things like col_set_str() and call_dissector() must not be within an if (tree)
> > > > {...} block.
> > > > 
> > > > (I have been unable to access the subversion archives to commit a change myself
> > > > since the recent site migration, so maybe someone else could fix this?)
> > > 
> > > Hi Chris,
> > > I have attached a fix ! If it is good for you, i commit !
> > 
> > Well, if tree is NULL, then pn532_tree will also be NULL, so there's no need to
> > make these calls either:
> > 
> > proto_tree_add_item(pn532_tree, hf_pn532_direction, tvb, 0, 1, ENC_NA);
> > etc.
> 
> Of course we might just consider eliminating the if (tree) checks altogether. 
> Jaap's recent comment #3 in the recently closed bug 6849 seems to indicate that
> this is his preference, and I've always wondered what the real performance
> improvement is myself.  Maybe it is worth doing, but those if (tree) checks
> have been the source of many bugs and I suspect there are still quite a few
> lingering.

This dissector basically does no more work when tree==NULL so I'd prefer to
just leave it in.  I just moved the if(tree) check a few lines down so the
col_set_str() is always called.

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.