snprintf will automatically write \0 at the end of the string, and the last one byte will be out of bound.
create a new func format_hex_arg with delimiter in utils, instead of chassisid_to string and format_hex_arg. Found in sanitize test. Signed-off-by: Changliang Wu <[email protected]> --- v2: create new function for hex string with delimiter --- lib/ofp-actions.c | 15 ++------------- lib/ofp-packet.c | 13 +------------ lib/ovs-lldp.c | 35 ++++++++++------------------------- lib/util.c | 13 +++++++++++++ lib/util.h | 4 ++++ tests/library.at | 4 ++++ tests/test-util.c | 20 ++++++++++++++++++++ 7 files changed, 54 insertions(+), 50 deletions(-) diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 102abbc23..d3c2195ac 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -1023,17 +1023,6 @@ parse_CONTROLLER(char *arg, const struct ofpact_parse_params *pp) return NULL; } -static void -format_hex_arg(struct ds *s, const uint8_t *data, size_t len) -{ - for (size_t i = 0; i < len; i++) { - if (i) { - ds_put_char(s, '.'); - } - ds_put_format(s, "%02"PRIx8, data[i]); - } -} - static void format_CONTROLLER(const struct ofpact_controller *a, const struct ofpact_format_params *fp) @@ -1063,7 +1052,7 @@ format_CONTROLLER(const struct ofpact_controller *a, } if (a->userdata_len) { ds_put_format(fp->s, "%suserdata=%s", colors.param, colors.end); - format_hex_arg(fp->s, a->userdata, a->userdata_len); + format_hex_arg(fp->s, a->userdata, a->userdata_len, "."); ds_put_char(fp->s, ','); } if (a->pause) { @@ -5960,7 +5949,7 @@ format_NOTE(const struct ofpact_note *a, const struct ofpact_format_params *fp) { ds_put_format(fp->s, "%snote:%s", colors.param, colors.end); - format_hex_arg(fp->s, a->data, a->length); + format_hex_arg(fp->s, a->data, a->length, "."); } static enum ofperr diff --git a/lib/ofp-packet.c b/lib/ofp-packet.c index 9485ddfc9..e1875c84b 100644 --- a/lib/ofp-packet.c +++ b/lib/ofp-packet.c @@ -916,17 +916,6 @@ ofputil_decode_packet_in_private(const struct ofp_header *oh, bool loose, return error; } -static void -format_hex_arg(struct ds *s, const uint8_t *data, size_t len) -{ - for (size_t i = 0; i < len; i++) { - if (i) { - ds_put_char(s, '.'); - } - ds_put_format(s, "%02"PRIx8, data[i]); - } -} - void ofputil_packet_in_private_format(struct ds *s, const struct ofputil_packet_in_private *pin, @@ -973,7 +962,7 @@ ofputil_packet_in_private_format(struct ds *s, if (public->userdata_len) { ds_put_cstr(s, " userdata="); - format_hex_arg(s, pin->base.userdata, pin->base.userdata_len); + format_hex_arg(s, pin->base.userdata, pin->base.userdata_len, "."); ds_put_char(s, '\n'); } diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c index 2d13e971e..8c9d7890a 100644 --- a/lib/ovs-lldp.c +++ b/lib/ovs-lldp.c @@ -101,21 +101,6 @@ static struct hmap *const all_mappings OVS_GUARDED_BY(mutex) = &all_mappings__; static struct lldp_aa_element_system_id system_id_null; -/* Convert an LLDP chassis ID to a string. - */ -static void -chassisid_to_string(uint8_t *array, size_t len, char **str) -{ - unsigned int i; - - *str = xmalloc(len * 3); - - for (i = 0; i < len; i++) { - snprintf(&(*str)[i * 3], 4, "%02x:", array[i]); - } - (*str)[(i * 3) - 1] = '\0'; -} - /* Find an Auto Attach mapping keyed by I-SID. */ static struct aa_mapping_internal * @@ -204,30 +189,30 @@ aa_print_element_status_port(struct ds *ds, struct lldpd_hardware *hw) sizeof port->p_element.system_id)) { const char *none_str = "<None>"; const char *descr = NULL; - char *id = NULL; - char *system; + struct ds id = DS_EMPTY_INITIALIZER; + struct ds system = DS_EMPTY_INITIALIZER; if (port->p_chassis) { if (port->p_chassis->c_id_len > 0) { - chassisid_to_string(port->p_chassis->c_id, - port->p_chassis->c_id_len, &id); + format_hex_arg(&id, port->p_chassis->c_id, + port->p_chassis->c_id_len, ":"); } descr = port->p_chassis->c_descr; } - chassisid_to_string((uint8_t *) &port->p_element.system_id, - sizeof port->p_element.system_id, &system); + format_hex_arg(&system, (uint8_t *) &port->p_element.system_id, + sizeof port->p_element.system_id, ":"); ds_put_format(ds, " Auto Attach Primary Server Id: %s\n", - id ? id : none_str); + id.length ? ds_cstr_ro(&id) : none_str); ds_put_format(ds, " Auto Attach Primary Server Descr: %s\n", descr ? descr : none_str); ds_put_format(ds, " Auto Attach Primary Server System Id: %s\n", - system); + id.length ? ds_cstr_ro(&system) : none_str); - free(id); - free(system); + ds_destroy(&id); + ds_destroy(&system); } } } diff --git a/lib/util.c b/lib/util.c index 890c606b5..fe178c995 100644 --- a/lib/util.c +++ b/lib/util.c @@ -38,6 +38,7 @@ #include "ovs-thread.h" #include "socket-util.h" #include "timeval.h" +#include "openvswitch/dynamic-string.h" #include "openvswitch/vlog.h" #ifdef HAVE_PTHREAD_SET_NAME_NP #include <pthread_np.h> @@ -1077,6 +1078,18 @@ free: return 0; } +/* Converts a uint8 array to a hex string. + * If delimiter is not NULL, it will be added between each hex character. */ +void +format_hex_arg(struct ds *s, const uint8_t *data, size_t len, char *delimiter) +{ + for (size_t i = 0; i < len; i++) { + if (i && delimiter) { + ds_put_format(s, "%s", delimiter); + } + ds_put_format(s, "%02" PRIx8, data[i]); + } +} /* Returns the current working directory as a malloc()'d string, or a null * pointer if the current working directory cannot be determined. */ diff --git a/lib/util.h b/lib/util.h index c1fd120bc..2e067107f 100644 --- a/lib/util.h +++ b/lib/util.h @@ -27,6 +27,7 @@ #include <stdlib.h> #include <string.h> #include "compiler.h" +#include "openvswitch/dynamic-string.h" #include "openvswitch/util.h" #if defined(__aarch64__) && __GNUC__ >= 6 #include <arm_neon.h> @@ -259,6 +260,9 @@ uintmax_t hexits_value(const char *s, size_t n, bool *ok); int parse_int_string(const char *s, uint8_t *valuep, int field_width, char **tail); +void +format_hex_arg(struct ds *s, const uint8_t *data, size_t len, char *delimiter); + const char *english_list_delimiter(size_t index, size_t total); char *get_cwd(void); diff --git a/tests/library.at b/tests/library.at index 82ac80a27..6fb0a31f9 100644 --- a/tests/library.at +++ b/tests/library.at @@ -249,6 +249,10 @@ AT_KEYWORDS([sat math sat_math]) AT_CHECK([ovstest test-util sat_math]) AT_CLEANUP +AT_SETUP([format_hex_arg]) +AT_CHECK([ovstest test-util format_hex_arg]) +AT_CLEANUP + AT_SETUP([snprintf]) AT_CHECK([ovstest test-util snprintf]) AT_CLEANUP diff --git a/tests/test-util.c b/tests/test-util.c index 5d88d38f2..22972639a 100644 --- a/tests/test-util.c +++ b/tests/test-util.c @@ -1183,6 +1183,25 @@ test_sat_math(struct ovs_cmdl_context *ctx OVS_UNUSED) } } +static void +test_format_hex_arg(struct ovs_cmdl_context *ctx OVS_UNUSED) +{ + struct ds ds = DS_EMPTY_INITIALIZER; + uint8_t array[6] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 }; + char str1[] = "01.02.03.04.05.06"; + char str2[] = "010203040506"; + + format_hex_arg(&ds, array, 6, "."); + ovs_assert(!strcmp(ds_cstr_ro(&ds), str1)); + + ds_clear(&ds); + + format_hex_arg(&ds, array, 6, NULL); + ovs_assert(!strcmp(ds_cstr_ro(&ds), str2)); + + ds_destroy(&ds); +} + #ifndef _WIN32 static void test_file_name(struct ovs_cmdl_context *ctx) @@ -1220,6 +1239,7 @@ static const struct ovs_cmdl_command commands[] = { {"ovs_scan", NULL, 0, 0, test_ovs_scan, OVS_RO}, {"snprintf", NULL, 0, 0, test_snprintf, OVS_RO}, {"sat_math", NULL, 0, 0, test_sat_math, OVS_RO}, + {"format_hex_arg", NULL, 0, 0, test_format_hex_arg, OVS_RO}, #ifndef _WIN32 {"file_name", NULL, 1, INT_MAX, test_file_name, OVS_RO}, #endif -- 2.43.5 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
