4:35 a.m.
I've started seeing a build failure on rawhide because of new wireshark
(3.6.0). The problem is that wmem_alloc() function which we use was
removed from public symbols starting with the 3.6.0 release in favor of
wmem_new(). However, as I was looking at the code I realized that
wireshark already has a function for what our code does and decided to
drop our implementation in favor of wireshark's.
Michal Prívozník (2):
wireshark: Switch to tvb_bytes_to_str()
wireshark: Drop needless comment in dissect_xdr_bytes()
tools/wireshark/src/packet-libvirt.c | 32 +++++++---------------------
1 file changed, 8 insertions(+), 24 deletions(-)
--
2.32.0
4:35 a.m.
New subject: [PATCH 1/2] wireshark: Switch to tvb_bytes_to_str()
When the dissector sees a byte sequence that is either an opaque
data (xdr_opaque) or a byte sequence (xdr_bytes) it formats the
bytes as a hex numbers using our own implementation. But
wireshark already provides a function for it: tvb_bytes_to_str().
NB, the reason why it returns a const string is so that callers
don't try to free it - the string is allocated using an allocator
which will decide when to free it.
The wireshark formatter was introduced in wireshark commit of
v1.99.2~479 and thus is present in the version we require at
least (2.6.0).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/wireshark/src/packet-libvirt.c | 30 ++++++++--------------------
1 file changed, 8 insertions(+), 22 deletions(-)
diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
index f43919b05d..cb922b8070 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -158,24 +158,6 @@ dissect_xdr_string(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int
hf,
}
}
-static const gchar *
-format_xdr_bytes(guint8 *bytes, guint32 length)
-{
- gchar *buf;
- guint32 i;
-
- if (length == 0)
- return "";
- buf = wmem_alloc(wmem_packet_scope(), length*2 + 1);
- for (i = 0; i < length; i++) {
- /* We know that buf has enough size to contain
- 2 * length + '\0' characters. */
- g_snprintf(buf, 2*(length - i) + 1, "%02x", bytes[i]);
- buf += 2;
- }
- return buf - length*2;
-}
-
static gboolean
dissect_xdr_opaque(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf,
guint32 size)
@@ -187,8 +169,10 @@ dissect_xdr_opaque(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int
hf,
val = g_malloc(size);
start = xdr_getpos(xdrs);
if ((rc = xdr_opaque(xdrs, (caddr_t)val, size))) {
- proto_tree_add_bytes_format_value(tree, hf, tvb, start, xdr_getpos(xdrs) -
start,
- NULL, "%s", format_xdr_bytes(val,
size));
+ gint len = xdr_getpos(xdrs) - start;
+ const char *s = tvb_bytes_to_str(wmem_packet_scope(), tvb, start, len);
+
+ proto_tree_add_bytes_format_value(tree, hf, tvb, start, len, NULL,
"%s", s);
} else {
proto_tree_add_item(tree, hf_libvirt_unknown, tvb, start, -1, ENC_NA);
}
@@ -207,8 +191,10 @@ dissect_xdr_bytes(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int
hf,
start = xdr_getpos(xdrs);
if (xdr_bytes(xdrs, (char **)&val, &length, maxlen)) {
- proto_tree_add_bytes_format_value(tree, hf, tvb, start, xdr_getpos(xdrs) -
start,
- NULL, "%s", format_xdr_bytes(val,
length));
+ gint len = xdr_getpos(xdrs) - start;
+ const char *s = tvb_bytes_to_str(wmem_packet_scope(), tvb, start, len);
+
+ proto_tree_add_bytes_format_value(tree, hf, tvb, start, len, NULL,
"%s", s);
/* Seems I can't call xdr_free() for this case.
It will raises SEGV by referencing out of bounds call stack */
free(val);
--
2.32.0
4:35 a.m.
New subject: [PATCH 2/2] wireshark: Drop needless comment in dissect_xdr_bytes()
In the dissect_xdr_bytes() there's a comment that the string
allocated by xdr_bytes() can't be freed using xdr_free(). Well,
that is expected because xdr_bytes() used plain calloc() AND the
string is not an XDR struct but plain 'char *' type. Passing it
to xdr_free() must result in weird things happening.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/wireshark/src/packet-libvirt.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
index cb922b8070..eeacbcdf0e 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -195,8 +195,6 @@ dissect_xdr_bytes(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf,
const char *s = tvb_bytes_to_str(wmem_packet_scope(), tvb, start, len);
proto_tree_add_bytes_format_value(tree, hf, tvb, start, len, NULL,
"%s", s);
- /* Seems I can't call xdr_free() for this case.
- It will raises SEGV by referencing out of bounds call stack */
free(val);
return TRUE;
} else {
--
2.32.0
6 a.m.
On Mon, Nov 29, 2021 at 11:35:48 +0100, Michal Privoznik wrote:
I've started seeing a build failure on rawhide because of new
wireshark
(3.6.0). The problem is that wmem_alloc() function which we use was
removed from public symbols starting with the 3.6.0 release in favor of
wmem_new(). However, as I was looking at the code I realized that
wireshark already has a function for what our code does and decided to
drop our implementation in favor of wireshark's.
Michal Prívozník (2):
wireshark: Switch to tvb_bytes_to_str()
wireshark: Drop needless comment in dissect_xdr_bytes()
tools/wireshark/src/packet-libvirt.c | 32 +++++++---------------------
1 file changed, 8 insertions(+), 24 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
1088
days inactive
1088
days old
3 comments
2 participants
participants (2)
-
Jiri Denemark -
Michal Privoznik