On Thu, 07 Apr 2005 10:29:41 -0400, "Ouellette, Jeremy J (N-Scientific
Research Corp)" <jeremy.j.ouellette@xxxxxxxx> wrote:
> Ronnie-
>
> Thanks for the feedback. Obviously some of the suggestions will be
> easier to implement than others -- I've already modified it so that
> offsets are passed by value, not address, and the style-related changes
> are simple as well.
nice.
> Are these all must-haves for this protocol to be considered for
> inclusion?
There are no musts as far as i know when it comes to ethereal.
When time permits some of the ethereal guys can give quick reviews
and comment on committed code.
There are no discussions among ethereal developer before checking anything in
If i request some changes before checking it in it only means i as an
individual requested them. if someone else thinks they are fine as
is and checks it in as is, so be it.
I am aware my suggestions are quite work intensive for your patch
but it would reduce future maintenance work on the dissector to reduce
the amount of special structures and code used.
If you really cant do those changes, do the changes that you can
(the passing offset by pointer is disgusting and must be changed) and
resubmit what you have.
Worst thing that can happen is that your dissector is different from
the others and thus suffer greater risk of bit-rot creeping in than
the normal ones.
Also change the function headers to stay on one line.
This
function(
type variable,
...
)
Thing reminds me too much of the bad old days with kr or kr
compatibility mode in compilers. It is also inconsistent with teh way
function headers are written in the rest of ethereal.
<crazy old mans rant>
Personally i hate proto_tree_add_text() and have often though about
rewriting all 700+ or so calls of this function so i can delete it so
that it will not be used any more.
Using proto_tree_Add_text() amkes the dissector sub-standard since
filtering is no longer possible. Removing this function completely
removes this shortcut.
maybe we need a coding style doco for ethereal?
</crazy old mans rant>
Please do the changes you can (offset change is mandatory as far as i
can say but i am just one individual == I will not check it in
without the offset change, others might) and resubmit.
> In particular, I realize that the overuse of
> proto_tree_add_text() isn't ideal and had noted it as such in my
> comments in packet-dis.c. However, I believe that in its current state,
> the dissector is already very useful to the simulation community, and
> adding it to the source tree would provide a mechanism to begin making
> your suggested changes as well as extending the dissector to cover
> additional PDU types as needed by other users.
>
> Thanks,
> Jeremy Ouellette
> Scientific Research Corporation
>
