From 22149c5523a77e642ec13d12064b2ccef29e51fb Mon Sep 17 00:00:00 2001 From: Evan Huus Date: Sat, 22 Feb 2014 08:39:57 -0500 Subject: [PATCH] TVB API deprecations and cleanup - rename tvb_length and similar to tvb_captured_length and similar; leave #defines in place for backwards-compat, but mark them clearly as deprecated in code comments and in checkAPI - remove tvb_get_string as C code and just leave a #define in place for backwards-compat; mark it clearly as deprecated in code comment and checkAPI - update READMEs and sample dissector for all of the above - while in the neighbourhood, make checkAPI skip (and warn) for missing files instead of bailing on the whole check, so subsequent files still get checked Change-Id: I32fc437896ca86ca73e9b49d5f50400adf8ec5ad Reviewed-on: https://code.wireshark.org/review/311 Reviewed-by: Evan Huus --- doc/README.dissector | 21 ++++++-------------- doc/packet-PROTOABBREV.c | 6 +++--- epan/tvbuff.c | 30 ++++++++++------------------ epan/tvbuff.h | 42 ++++++++++++++++++++++++---------------- tools/checkAPIs.pl | 21 ++++++++++++++------ 5 files changed, 59 insertions(+), 61 deletions(-) diff --git a/doc/README.dissector b/doc/README.dissector index 4db96d4623..59b0900b48 100644 --- a/doc/README.dissector +++ b/doc/README.dissector @@ -249,21 +249,12 @@ void tvb_get_guid(tvbuff_t *tvb, const gint offset, e_guid_t *guid, const guint String accessors: -guint8 *tvb_get_string(wmem_allocator_t *scope, tvbuff_t *tvb, const gint offset, const gint length); guint8 *tvb_get_string_enc(wmem_allocator_t *scope, tvbuff_t *tvb, const gint offset, const gint length, const guint encoding); -Returns a null-terminated buffer containing data from the specified -tvbuff, starting at the specified offset, and containing the specified -length worth of characters (the length of the buffer will be length+1, -as it includes a null character to terminate the string). - -tvb_get_string() returns a buffer allocated by g_malloc() if scope is set -to NULL (in that case memory must be explicitely freed), or with the -allocator lifetime if scope is not NULL. - -tvb_get_string_enc() is a version of tvb_get_string() that takes a -string encoding as an argument. See below for a list of encoding values -for strings. +Returns a null-terminated buffer allocated from the specified scope, containing +data from the specified tvbuff, starting at the specified offset, and containing +the specified length worth of characters. Reads data in the specified encoding +and produces UTF-8 in the buffer. See below for a list of input encoding values. guint8 *tvb_get_stringz(wmem_allocator_t *scope, tvbuff_t *tvb, const gint offset, gint *lengthp); guint8 *tvb_get_stringz_enc(wmem_allocator_t *scope, tvbuff_t *tvb, const gint offset, gint *lengthp, const guint encoding); @@ -2005,7 +1996,7 @@ dissect_ipx(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) /* Calculate the available data in the packet, set this to -1 to use all the data in the tv_buffer */ - available_length = tvb_length(tvb) - IPX_HEADER_LEN; + available_length = tvb_captured_length(tvb) - IPX_HEADER_LEN; /* Create the tvbuffer for the next dissector */ next_tvb = tvb_new_subset(tvb, IPX_HEADER_LEN, @@ -2886,7 +2877,7 @@ reference to a callback which will be called with reassembled data: { tcp_dissect_pdus(tvb, pinfo, tree, dns_desegment, 2, get_dns_pdu_len, dissect_dns_tcp_pdu, data); - return tvb_length(tvb); + return tvb_captured_length(tvb); } (The dissect_dns_tcp_pdu function acts similarly to dissect_dns_udp.) diff --git a/doc/packet-PROTOABBREV.c b/doc/packet-PROTOABBREV.c index 1e4c9975d6..c1aaba8d36 100644 --- a/doc/packet-PROTOABBREV.c +++ b/doc/packet-PROTOABBREV.c @@ -97,7 +97,7 @@ dissect_PROTOABBREV(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, */ /* Check that there's enough data */ - if (tvb_length(tvb) < PROTOABBREV_MIN_LENGTH) + if (tvb_reported_length(tvb) < PROTOABBREV_MIN_LENGTH) return 0; /* Fetch some values from the packet header using tvb_get_*(). If these @@ -181,8 +181,8 @@ dissect_PROTOABBREV(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, * README.dissector for more information. */ /* Return the amount of data this dissector was able to dissect (which may - * or may not be the entire packet as we return here). */ - return tvb_length(tvb); + * or may not be the total captured packet as we return here). */ + return tvb_captured_length(tvb); } /* Register the protocol with Wireshark. diff --git a/epan/tvbuff.c b/epan/tvbuff.c index a0371a7423..123e7ec473 100644 --- a/epan/tvbuff.c +++ b/epan/tvbuff.c @@ -304,7 +304,7 @@ tvb_new_octet_aligned(tvbuff_t *tvb, guint32 bit_offset, gint32 no_of_bits) right = 8 - left; /* for right-shifting */ if (no_of_bits == -1) { - datalen = tvb_length_remaining(tvb, byte_offset); + datalen = tvb_captured_length_remaining(tvb, byte_offset); remaining_bits = 0; } else { datalen = no_of_bits >> 3; @@ -326,7 +326,7 @@ tvb_new_octet_aligned(tvbuff_t *tvb, guint32 bit_offset, gint32 no_of_bits) * if non extra byte is available, the last shifted byte requires * special treatment */ - if (tvb_length_remaining(tvb, byte_offset) > datalen) { + if (tvb_captured_length_remaining(tvb, byte_offset) > datalen) { data = tvb_get_ptr(tvb, byte_offset, datalen + 1); /* Do this allocation AFTER tvb_get_ptr() (which could throw an exception) */ @@ -390,7 +390,7 @@ tvb_clone(tvbuff_t *tvb) } guint -tvb_length(const tvbuff_t *tvb) +tvb_captured_length(const tvbuff_t *tvb) { DISSECTOR_ASSERT(tvb && tvb->initialized); @@ -398,7 +398,7 @@ tvb_length(const tvbuff_t *tvb) } gint -tvb_length_remaining(const tvbuff_t *tvb, const gint offset) +tvb_captured_length_remaining(const tvbuff_t *tvb, const gint offset) { guint abs_offset, rem_length; int exception; @@ -413,7 +413,7 @@ tvb_length_remaining(const tvbuff_t *tvb, const gint offset) } guint -tvb_ensure_length_remaining(const tvbuff_t *tvb, const gint offset) +tvb_ensure_captured_length_remaining(const tvbuff_t *tvb, const gint offset) { guint abs_offset, rem_length; int exception; @@ -2474,16 +2474,6 @@ tvb_get_string_enc(wmem_allocator_t *scope, tvbuff_t *tvb, const gint offset, return strbuf; } -/* - * Get an ASCII string; this should not be used in new code. - */ -guint8 * -tvb_get_string(wmem_allocator_t *scope, tvbuff_t *tvb, const gint offset, - const gint length) -{ - return tvb_get_ascii_string(scope, tvb, offset, length); -} - /* * These routines are like the above routines, except that they handle * null-terminated strings. They find the length of that string (and @@ -2949,7 +2939,7 @@ tvb_find_line_end(tvbuff_t *tvb, const gint offset, int len, gint *next_offset, guchar found_needle = 0; if (len == -1) - len = tvb_length_remaining(tvb, offset); + len = tvb_captured_length_remaining(tvb, offset); /* * XXX - what if "len" is still -1, meaning "offset is past the * end of the tvbuff"? @@ -3065,7 +3055,7 @@ tvb_find_line_end_unquoted(tvbuff_t *tvb, const gint offset, int len, gint *next int linelen; if (len == -1) - len = tvb_length_remaining(tvb, offset); + len = tvb_captured_length_remaining(tvb, offset); /* * XXX - what if "len" is still -1, meaning "offset is past the * end of the tvbuff"? @@ -3203,7 +3193,7 @@ tvb_skip_wsp(tvbuff_t *tvb, const gint offset, const gint maxlength) guint8 tempchar; /* Get the length remaining */ - tvb_len = tvb_length(tvb); + tvb_len = tvb_captured_length(tvb); end = offset + maxlength; if (end >= tvb_len) { @@ -3238,7 +3228,7 @@ tvb_skip_guint8(tvbuff_t *tvb, int offset, const int maxlength, const guint8 ch) int end, tvb_len; /* Get the length remaining */ - tvb_len = tvb_length(tvb); + tvb_len = tvb_captured_length(tvb); end = offset + maxlength; if (end >= tvb_len) end = tvb_len; @@ -3294,7 +3284,7 @@ tvb_bcd_dig_to_wmem_packet_str(tvbuff_t *tvb, const gint offset, const gint len, dgt = &Dgt1_9_bcd; if (len == -1) { - length = tvb_length(tvb); + length = tvb_captured_length(tvb); if (length < offset) { return ""; } diff --git a/epan/tvbuff.h b/epan/tvbuff.h index be7db043f3..9e15612105 100644 --- a/epan/tvbuff.h +++ b/epan/tvbuff.h @@ -221,18 +221,29 @@ WS_DLL_PUBLIC tvbuff_t *tvb_new_composite(void); WS_DLL_PUBLIC void tvb_composite_finalize(tvbuff_t *tvb); -/* Get total length of buffer */ -WS_DLL_PUBLIC guint tvb_length(const tvbuff_t *tvb); +/* Get amount of captured data in the buffer (which is *NOT* necessarily the + * length of the packet). You probably want tvb_reported_length instead. */ +WS_DLL_PUBLIC guint tvb_captured_length(const tvbuff_t *tvb); + +/* DEPRECATED, do not use in new code, call tvb_captured_length directly! */ +#define tvb_length tvb_captured_length /** Computes bytes to end of buffer, from offset (which can be negative, * to indicate bytes from end of buffer). Function returns 0 if offset is - * either at the end of the buffer or out of bounds. No exception is thrown. */ -WS_DLL_PUBLIC gint tvb_length_remaining(const tvbuff_t *tvb, const gint offset); + * either at the end of the buffer or out of bounds. No exception is thrown. + * You probably want tvb_reported_length_remaining instead. */ +WS_DLL_PUBLIC gint tvb_captured_length_remaining(const tvbuff_t *tvb, const gint offset); + +/* DEPRECATED, do not use in new code, call tvb_captured_length_remaining directly! */ +#define tvb_length_remaining tvb_captured_length_remaining /** Same as above, but throws an exception if the offset is out of bounds. */ -WS_DLL_PUBLIC guint tvb_ensure_length_remaining(const tvbuff_t *tvb, +WS_DLL_PUBLIC guint tvb_ensure_captured_length_remaining(const tvbuff_t *tvb, const gint offset); +/* DEPRECATED, do not use in new code, call tvb_ensure_captured_length_remaining directly! */ +#define tvb_ensure_length_remaining tvb_ensure_captured_length_remaining + /* Checks (w/o throwing exception) that the bytes referred to by * 'offset'/'length' actually exist in the buffer */ WS_DLL_PUBLIC gboolean tvb_bytes_exist(const tvbuff_t *tvb, const gint offset, @@ -472,23 +483,20 @@ extern gchar *tvb_format_stringzpad_wsp(tvbuff_t *tvb, const gint offset, * * Throws an exception if the tvbuff ends before the string does. * - * tvb_get_string() handles 7-bit ASCII strings, with characters - * with the 8th bit set are converted to the - * Unicode REPLACEMENT CHARACTER. + * Takes a string encoding as well, and converts to UTF-8 from the encoding, + * possibly mapping some characters to the Unicode REPLACEMENT CHARACTER. * - * tvb_get_string_enc() takes a string encoding as well, and converts to UTF-8 - * from the encoding, possibly mapping some characters - * to the REPLACEMENT CHARACTER. - * - * If scope is set to NULL it is the user's responsibility to g_free() - * the memory allocated by tvb_memdup(). Otherwise memory is - * automatically freed when the scope lifetime is reached. + * If scope is set to NULL it is the user's responsibility to wmem_free() + * the memory allocated. Otherwise memory is automatically freed when the scope + * lifetime is reached. */ -WS_DLL_PUBLIC guint8 *tvb_get_string(wmem_allocator_t *scope, tvbuff_t *tvb, - const gint offset, const gint length); WS_DLL_PUBLIC guint8 *tvb_get_string_enc(wmem_allocator_t *scope, tvbuff_t *tvb, const gint offset, const gint length, const guint encoding); +/* DEPRECATED, do not use in new code, call tvb_get_string_enc directly! */ +#define tvb_get_string(SCOPE, TVB, OFFSET, LENGTH) \ + tvb_get_string_enc(SCOPE, TVB, OFFSET, LENGTH, ENC_ASCII) + /** * Given a tvbuff, a bit offset, and a number of characters, allocate * a buffer big enough to hold a non-null-terminated string of no_of_chars diff --git a/tools/checkAPIs.pl b/tools/checkAPIs.pl index 2ab20c3e37..e21f75323d 100755 --- a/tools/checkAPIs.pl +++ b/tools/checkAPIs.pl @@ -124,10 +124,16 @@ my %APIs = ( '_snwprintf' # use StringCchPrintf ] }, - ### Deprecated emem functions (use wmem instead!) - # These will become errors once they've been removed from all the - # existing code - 'emem' => { 'count_errors' => 0, 'functions' => [ + ### Soft-Deprecated functions that should not be used in new code but + # have not been entirely removed from old code. These will become errors + # once they've been removed from all existing code. + 'soft-deprecated' => { 'count_errors' => 0, 'functions' => [ + 'tvb_length', # replaced with tvb_captured_length + 'tvb_length_remaining', # replaced with tvb_captured_length_remaining + 'tvb_ensure_length_remaining', # replaced with tvb_ensure_captured_length_remaining + 'tvb_get_string', # replaced with tvb_get_string_enc + + # wmem calls should replace all emem calls (see doc/README.wmem) 'ep_alloc', 'ep_new', 'ep_alloc0', @@ -329,7 +335,7 @@ my %APIs = ( ); -my @apiGroups = qw(prohibited deprecated emem); +my @apiGroups = qw(prohibited deprecated soft-deprecated); # Deprecated GTK+ (and GDK) functions/macros with (E)rror or (W)arning flag: # (The list is based upon the GTK+ 2.24.8 documentation; @@ -1946,7 +1952,10 @@ while ($_ = $ARGV[0]) my @foundAPIs = (); my $line; - die "No such file: \"$filename\"" if (! -e $filename); + if (! -e $filename) { + warn "No such file: \"$filename\""; + next; + } # delete leading './' $filename =~ s{ ^ \. / } {}xo;