Changliang Wu <[email protected]> writes:
> 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]);
> + }
> +}
We have a ds_put_hex and ds_put_hex_dump already, so I wonder if we
should make this part of dynamic string library, ex:
ds_put_hex_bytes_with_delimiter()
Then still do the format_hex_arg cleanup as well. WDYT?
Ilya?
>
> /* 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
