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
> >
> >
> >
> >
> >
> >
>
