Peter Wu
changed
bug 9498
Comment # 10
on bug 9498
from Peter Wu
(In reply to comment #7)
> Created attachment 12250 [details]
> ssl: move TLS ext dissect code to packet-ssl-utils.c
>
> This move the code used to parse the TLS extension into packet-
> ssl-utils.c and adds an architecture that this code could be used by
> dtls in the next patch. This patch should not change anything in the
> functionality.
>
> TODO: this should give a compile error if ssl_hf_list_t was not dully filled.
The struct initialization is just boilerplate code right now. What about
dropping the extra hf_ssl_* stuff such as:
.hf_hs_exts_len = &hf_ssl_handshake_extensions_len,
and directly use the struct for storing header fields:
struct {
gint hf_hs_exts_len; /* note: removed pointer */
gint hf_hs_ext_alpn_len;
/* etc */
} ssl_hf_list_t;
static struct ssl_hf_list_t ssl_hf;
...
/* note: removed indirection */
proto_tree_add_uint(tree, hf->hf_hs_exts_len
According to proto_register_field_array, 0 is also a valid "uninitialized"
value besides -1, so advantage could be taken of that to avoid initialising it
to -1. For the ett and ei, explicit initialization is necessary, that could be
done by a static boolean "ssl_hf_is_initialized" to avoid re-initialization
later.
Advantage: less boiler-plate code in packet-ssl.c and packet-dtls.c, no need to
worry about uninitialized vars and slightly less memory usage due to removal of
pointers.
The header fields themselves (hf) in the init code are also shared between DTLS
and SSL, I wonder if it is possible to move them in a common initializator
function? Even if it needs macros, DRY is a good principle to follow here.
I quickly scanned through your attachment 12250 [details] patch, diffing the reverse of
packet-ssl.c with packet-ssl-utils.c and found no big issues (most code is not
touched except for renaming). To be sure, at least the current small TLS test
should pass and valgrind should not complain more.
You are receiving this mail because:
- You are watching all bug changes.
