Ethereal-dev: Re: [Ethereal-dev] [PATCH] packet-dcerpc.c: clamp to tvb length and display mult
* On Friday 2004-12-03 at 10:12:09 -0800, Guy Harris wrote:
> You're assuming, perhaps incorrectly, that the people in question *have*
> a view. There are places in dissectors where padding is put into the
> protocol tree, and places where it's not; I'm asking the question to see
> what people in general think. The question came up because you added it
> only in one place - I didn't see why that particular place was any
> different from the other places in the DCE RPC dissector where padding
> was added, so if it belonged there, it presumably belonged in other places.
I see. Would there be a limited opportunity to automate this?
Otherwise I can see how implementing it could turn out to be a huge
janitorial-type job.
I will say this. As an user of Ethereal, I often rely on it to learn
about protocols in a practical/applied fashion. That may be laziness on
my part (to not go straight for the protocol specification), but somehow
I'm sure I'm not alone in doing so. This is why I would advocate trying
to account for every octet in a packet; no long explanations or tutorials,
just a short label (such as specifying to how many bits the alignment
is meant), and no burden to have it done in every dissector there is if
nobody volunteers (do we need perfection at once?).
> "The subject" meaning moving the multi-line display stuff into the
> generic string code? I can't speak for other project leaders, but
> that's where I think it belongs, so I'd say I'm interested in it.
It would also be a good opportunity, at the same time, to look at what
to do with a string that has been deemed (individual dissector's call,
in my opinion) to be UTF-8. Some mailing list participants have recently
indicated interest in that, and there are some problems inherent to
malformed UTF-8 and Unicode that a program such as Ethereal should
especially be robust against.
> I don't think it's that complicated to provide. I would be tempted not
> to make it apply to all strings, as, if a string *doesn't* contain any
> newlines, I don't think there should be a subtree for it,
The subtree and its being collapsed or not (and the variable that
remembers that fact) was in fact the main problem I anticipated.
No subtree, problem solved, but I'm not sure that would always be the
best way to go.
> so the code
> would have to scan all strings, and I'd prefer not to spend CPU time on
> scanning strings that are for fields that aren't likely to contain
> newlines - it sounds as if the string in question is a message to be
> displayed in a popup on the user's machine - although if the extra CPU
> time doesn't slow Ethereal down significantly, perhaps it should apply
> to all strings. Perhaps it should be done for all strings, and then
> changed to have the field specify whether to do it or not if a lot of
> CPU time is used.
I agree that CPU usage is a concern, but on the other hand,
expected/average (in a statistical sense) performance would not
necessarily be degraded significantly if in practice most strings without
newlines are actual quite short (say < 80 characters).
Providing a simple boolean flag somewhere, to be set at the discretion
of the dissector, could be the simplest way to decide whether a string
should be scanned.
> > Do my contributions to it stand an equal chance of being
> > considered in a timely fashion and eventually accepted (once
> > revised)?
>
> How quickly changes get considered depend on the availability of time to
> consider them, the complexity of the changes, the depth of the issues
> they raise - and whether the issues happen to get noticed, so for better
> or worse, it can be somewhat random. At this point, I'll look at your
> changes, time permitting, and accept them if I think they're reasonable.
Ok. That's all I ask for. I plan to reply to Richard's message in a
few minutes and to address the reasons for my unusual level of edginess
there; please read it (if you care).
