Wireshark · Wireshark-dev: Re: [Wireshark-dev] Source code for ACN (ANSI BSR E1.17) Dissector

["ronnie sahlberg" <ronniesahlberg@xxxxxxxxx>]

Please also provide a patch for the AUTHORS file   so that we get it
right and we have someone to blame if we forget someone :-)



On 10/25/06, ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote:
> warnings for unused parameters are fixed easily by adding a _U_ to the
> function arguments.
>
> I.e.  foo(...,  int not_used _U_ , ...){
>
>
> On 10/25/06, ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote:
> > Since it wont go into the next release on monday anyway (already branched
> > off)
> >
> >
> > It produces a lot of compiler warnings when compiling with gcc :
> >
> >
> > packet-acn.c: In function `dissect_acn_heur':
> > packet-acn.c:734: warning: implicit declaration of function `memcmp'
> > packet-acn.c: In function `acn_add_channel_owner_info_block':
> > packet-acn.c:762: warning: unused parameter `tree'
> > packet-acn.c: In function `acn_add_channel_member_info_block':
> > packet-acn.c:797: warning: unused parameter `tree'
> > packet-acn.c: In function `acn_add_expiry':
> > packet-acn.c:836: warning: unused parameter `pinfo'
> > packet-acn.c: In function `acn_add_channel_parameter':
> > packet-acn.c:851: warning: unused parameter `pinfo'
> > packet-acn.c: In function `acn_add_cid':
> > packet-acn.c:874: warning: unused parameter `pinfo'
> > packet-acn.c: In function `acn_add_address':
> > packet-acn.c:967: warning: int format, pointer arg (arg 3)
> > packet-acn.c:967: warning: too many arguments for format
> > packet-acn.c:910: warning: unused parameter `pinfo'
> > packet-acn.c: In function `acn_add_dmp_address_type':
> > packet-acn.c:977: warning: unused parameter `pinfo'
> > packet-acn.c: In function `acn_add_dmp_address':
> > packet-acn.c:1018: warning: left-hand operand of comma expression has no
> > effect
> > packet-acn.c:1018: warning: statement with no effect
> > packet-acn.c:1023: warning: left-hand operand of comma expression has no
> > effect
> > packet-acn.c:1023: warning: statement with no effect
> > packet-acn.c:1000: warning: unused parameter `pinfo'
> > packet-acn.c: In function `acn_add_dmp_data':
> > packet-acn.c:1170: warning: unused parameter `pinfo'
> > packet-acn.c: In function `acn_add_dmp_reason_codes':
> > packet-acn.c:1427: warning: unused variable `ok_to_process'
> > packet-acn.c:1417: warning: unused parameter `pinfo'
> > packet-acn.c: In function `dissect_acn_dmp_pdu':
> > packet-acn.c:1563: warning: missing braces around initializer
> > packet-acn.c:1563: warning: (near initialization for `adt.<anonymous>')
> > packet-acn.c:1564: warning: missing braces around initializer
> > packet-acn.c:1564: warning: (near initialization for `adt2.<anonymous>')
> > packet-acn.c:1559: warning: unused variable `param_tree'
> > packet-acn.c:1554: warning: `address_count' might be used
> > uninitialized in this function
> > packet-acn.c: In function `dissect_acn_sdt_wrapped_pdu':
> > packet-acn.c:1916: warning: unused variable `param_tree'
> > packet-acn.c: In function `dissect_acn_dmx_data_pdu':
> > packet-acn.c:2220: warning: missing braces around initializer
> > packet-acn.c:2220: warning: (near initialization for `adt.<anonymous>')
> > packet-acn.c:2217: warning: unused variable `addr_tree'
> > packet-acn.c: At top level:
> > packet-acn.c:153: warning: `hf_acn_sdt_pdu' defined but not used
> > packet-acn.c:162: warning: `hf_acn_dmx_dmp_vector' defined but not used
> >
> > Can you look into eliminating these.
> >
> >
> > Also, while not a real issue, but in most dissector helpers we use we
> > return the new value of offset and not the number of bytes consumed.
> > Could you change your helpers to do this?
> > so that after calling them it would be
> >  offset=foo(..., offset, ...);
> >
> > instead of
> >   offset += foo(...);
> >
> >
> > This would make it much more inline with other dissectors and reduce
> > our maintenance workload of this dissector (since it is currently
> > different to most others).
> >
> >
> >
> > We also prefer (for consistency) that
> > the functions :
> > proto_register_PROTO()
> > and
> > proto_reg_handoff_PROTO()
> > are the last two functions defined in the fiel and not the first two ones
> > (so that ESC-> will bring you straight there   unless your editor is
> > "broken")
> >
> >
> >
> > We do suffer in wireshark from port collissions due to the number of
> > protocols we support.
> > So a port number is not really enough for us to identify a protocol.
> > Can you make dissect_acn() do some heuristics and return FALSE if it
> > didnt really look like ACN in the first place?
> > This would reduce the probability for false ACN dissection for those
> > users that have set an ACN port and forgotten about it.
> > I.e. make dissect_acn() a new style dissector that can refuse the
> > packet by returning FALSE and return TRUE meaning : yes this was one
> > of mine and i did dissect it.
> >
> > The sharper the heuristic filter is the better.
> >
> >
> > best regards
> > ronnie s
> >
> >
> >
> >
> > On 10/25/06, Bill Florac <bill.florac@xxxxxxxxxxxxxx> wrote:
> > > Ronnie,
> > >
> > > Correct files are attached.
> > >
> > > I created an ACN wiki page and uploaded a small capture file. (More to
> > > follow)
> > >
> > > Thanks for your time.
> > >
> > > Bill
> > >
> > >
> > >
> > > ________________________________
> > >
> > > From: wireshark-dev-bounces@xxxxxxxxxxxxx on behalf of ronnie sahlberg
> > > Sent: Tue 10/24/2006 5:24 PM
> > > To: Developer support list for Wireshark
> > > Subject: Re: [Wireshark-dev] Source code for ACN (ANSI BSR E1.17)
> > Dissector
> > >
> > >
> > > Hi,
> > >
> > > Can you try avoiding functions that allocate memory that has to be
> freed?
> > > We have ephemeral memory allocators for this purpose.
> > > I.e.   tvb_get_ephemeral_string instead of tvb_get_string+gfree
> > >
> > > There are some // comments in the source. Please replace with C comments
> > >
> > >
> > > Can you also provide a few small example captures to test with and a
> nice
> > > page on the WIKI for this protocol?
> > >
> > >
> > > Apart from that it looks good at a first glance.
> > >
> > >
> > >
> > >
> > >
> > >
> > > On 10/25/06, Bill Florac <bill.florac@xxxxxxxxxxxxxx> wrote:
> > >
> > >  Developers,
> > >
> > >  Please consider adding the attached source code for dissecting the
> > > "Architecture for Control Networks", or "ACN" to the Wireshark build. We
> > > have done a fair amount of internal testing and there a no known bugs. I
> > > think we complied with all the source code format requirement but if
> not,
> > > just let me know and I will gladly fix them. Once posted, I will also
> > submit
> > > capture files samples.
> > >
> > >  It was last tested on build 19109. It was tested as both a built in
> > > dissector and as a plug-in.  As requested (by Jaap) I am only submitting
> > the
> > > files for a built-in variant.  There is an existing ACN plug-in that
> > should
> > > be removed.
> > >
> > >  Dissector also includes an option to enable an extension protocol
> called
> > > Streaming DMX (ANSI BSR E1.31).  This extension has not been formally
> > > approved by ANSI yet and is subject to change..
> > >
> > >  <<packet-acn.c>> <<packet-acn.h>>
> > >  Bill Florac
> > >  Senior Technical Product Specialist
> > >  bill.florac@xxxxxxxxxxxxxx
> > >  Electronic Theatre Controls, Inc.
> > >  3031 Pleasant View Rd .
> > >  Middleton, WI 53562
> > >  608-831-4116 (corp. phone)
> > >
> > >
> > >
> > >  _______________________________________________
> > >  Wireshark-dev mailing list
> > >  Wireshark-dev@xxxxxxxxxxxxx
> > >  http://www.wireshark.org/mailman/listinfo/wireshark-dev
> > >
> > >
> > >
> > >
> > >
> > >
> >