[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wireshark-dev
Subject:    Re: [Wireshark-dev] FT_BYTES hf with len==0
From:       Martin Kaiser <lists () kaiser ! cx>
Date:       2013-12-20 17:25:19
Message-ID: 20131220172519.GA4881 () reykholt ! kaiser ! cx
[Download RAW message or body]

Thus wrote Martin Kaiser (lists@kaiser.cx):

> > > 5392             label_fill(label_str, 0, hfinfo,
> > > 5393                             (bytes) ? bytes_to_str(bytes, \
> > > fvalue_length(&fi->value)) : "<MISSING>"); 5394             break;

> > > It'd be good to make them consistent, allow empty bytes (+1 from me) or assert \
> > > in both place.

> > ok, I'll change this.

> done in r54290.

I've just found one more issue with empty FT_BYTES:
When I click on an empty FT_BYTES element in the tree and select Apply
as Filter / Selected, wireshark creates a filter expression

<filter name>==

that results in a syntax error.

This is created by construct_match_selected_string(), which calls
fvalue_to_string_repr() and uses the fvalue's string representation by
default.

One way to resolve this would be to change an empty FT_BYTES' string
representation from <nothing> to "" (as <filter name>=="" is a valid
filter expression). See the attached patch.

Is that ok or does it break someone else's code? Should I check for
FTREPR_DFILTER and modify the representation for empty FT_BYTES only in
this case?


["bytes_repr.patch" (text/x-diff)]

commit f7aeaec74abab8e11165ea742778d4875100cfac
Author: Martin Kaiser <martin@reykholt.kaiser.cx>
Date:   Fri Dec 20 17:31:21 2013 +0100

    change the string representation of an empty FT_BYTES fvalue to ""
    this creates a correct filter expression for Apply as Filter / Selected, etc.

diff --git a/epan/ftypes/ftype-bytes.c b/epan/ftypes/ftype-bytes.c
index de7adac..5bcb20d 100644
--- a/epan/ftypes/ftype-bytes.c
+++ b/epan/ftypes/ftype-bytes.c
@@ -63,9 +63,8 @@ static int
 bytes_repr_len(fvalue_t *fv, ftrepr_t rtype _U_)
 {
 	if (fv->value.bytes->len == 0) {
-		/* Empty array of bytes, so the representation
-		 * is an empty string. */
-		return 0;
+		/* Empty array of bytes, the representation is two double quotes */
+		return 2;
 	} else {
 		/* 3 bytes for each byte of the byte "NN:" minus 1 byte
 		 * as there's no trailing ":". */
@@ -143,6 +142,11 @@ bytes_to_repr(fvalue_t *fv, ftrepr_t rtype _U_, char *buf)
 	c = fv->value.bytes->data;
 	write_cursor = buf;
 
+	if (fv->value.bytes->len==0) {
+		sprintf(write_cursor, "\"\"");
+		return;
+	}
+
 	for (i = 0; i < fv->value.bytes->len; i++) {
 		if (i == 0) {
 			sprintf(write_cursor, "%02x", *c++);


___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@wireshark.org?subject=unsubscribe

[prev in list] [next in list] [prev in thread] [next in thread]