Hi,
the dissect_ber_named_bistring32() could be done. I have no objection to it.
I will look at it when I have time.
Maybe, my approach is too universal because I make packet-ber changes
together with the "ASN.1 to Ethereal dissector" compiler.
If you are interested in it I can send you current (but very preliminary) version.
Maybe, I hav not understood all well (as English in not my native language)
but the new dissect_ber_octet_string() does not require octet string containing ASN.1/BER
encoded data.
I try to keep all functions with common format, i.e. the last parameter should be
dissect_ber_...(..., value_type *value). And the best value type for OCTET/BIT STRING
seems to be tvbuff_t.
But I could make version with callback if it help you. E.g.:
int dissect_ber_octet_string_wcb(..., ber_callback func) {
tvbuff_t *next_tvb;
offset = dissect_ber_octet_string(..., (func)?&next_tvb:NULL);
if (func && (tvb_length(next_tvb)>0))
func(pinfo, tree, next_tvb, 0);
}
Regards,
Tomas
Ronnie Sahlberg wrote:
Hi,
Thanks for the interest in packet-ber.c.
I looked at the patch and it is really a step in the right direction.
The handling of bitstrings looks good but why not take it a few steps even
further:
It should be possible to make it even simpler while you are cleaning it up
properly anyway.
Change KDCOptions_bits to a NULL terminated array of pointers to gint :
so it becomes something like
static (*gint)[] = {
&hf_...,
...
NULL
};
Then change the call to :
offset=dissect_ber_named_bitstring32(FALSE, pinfo, tree, tvb, offset,
KDCOptions_bits, hf_krb_KDCOptions, ett_krb_KDC_Options, NULL);
and make dissect_ber_named_bistring32()
1, handle the bits in order until we come to the NULL entry.
2, the text to use to puch up on the expansion line can be found using
proto_registrar_get_nth(hf_index)->name (or is it abbrev?) so we
dont need to specify it a second time in the array as in your patch.
3, the dissect_ber_named_bistring32() should also for each hf_index chech
that the type is FT_BOOLEAN and that the number of bits was set to
32 if not do a g_assert() so we find the bug quickly.
I.e. since most bistrings are going to be named bistrings of length 32 bits,
why not make a special helper for that case
and let the other special cases either provide a new helper or do things
manually.
(in my new kerberos patch there is another bitstring (from inside the
encryted blobs))
The changes to the octetstring stuff, i have to admit i did not look too
closely on it and might be wrong
but Im not sure i like that.
The octet string thing really should take a callback function to call for
the content of the bitstring.
Not everything inside kerberos bitstrings are BER_APPLICATION, hey, not
everything that we need to pass to a subdissector is even ASN.1/BER encoded.
See in my latest kerberos patch from a day or two ago. Inside the PAC
structure there are octet strings that contain NDR encoded data. No
ASN.1 stuff at all.
Would you consider looking at making the bistring handling even simpler
while you are at cleaning ber up?
Could you also look at porting my packet-kerberos.c thing over to your new
api to check that the octetstring changes does not break it.
