On 11 Sep 2025, at 10:41, Changliang Wu wrote:
Thanks for following up on this patch. We would still love a short commit
message here.
One more comment below.
> Signed-off-by: Changliang Wu <changliang...@smartx.com>
> ---
> lib/colors.c | 2 +-
> lib/lldp/lldp.c | 7 ++++---
> lib/util.h | 6 ++++++
> 3 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/lib/colors.c b/lib/colors.c
> index 13456445e..09cf07847 100644
> --- a/lib/colors.c
> +++ b/lib/colors.c
> @@ -117,7 +117,7 @@ colors_parse_from_env(const struct color_key color_dic[])
> token != NULL;
> token = strsep(&s, ":")) {
> char *name = strsep(&token, "=");
> - for (char *ptr = token; ptr != NULL && *ptr != '\0'; ptr++) {
> + for (char *ptr = token; nullable_string_not_empty(ptr); ptr++) {
> /* We accept only decimals and ';' for color marker. */
> if (*ptr == ';' || (*ptr >= '0' && *ptr <= '9')) {
> continue;
> diff --git a/lib/lldp/lldp.c b/lib/lldp/lldp.c
> index 6fdcfef56..570c79e13 100644
> --- a/lib/lldp/lldp.c
> +++ b/lib/lldp/lldp.c
> @@ -28,6 +28,7 @@
> #include "compiler.h"
> #include "dp-packet.h"
> #include "packets.h"
> +#include "util.h"
>
> VLOG_DEFINE_THIS_MODULE(lldp);
>
> @@ -191,14 +192,14 @@ lldp_send(struct lldpd *global OVS_UNUSED,
> lldp_tlv_end(p, start);
>
> /* System name */
> - if (chassis->c_name && *chassis->c_name != '\0') {
> + if (nullable_string_not_empty(chassis->c_name)) {
> lldp_tlv_start(p, LLDP_TLV_SYSTEM_NAME, &start);
> dp_packet_put(p, chassis->c_name, strlen(chassis->c_name));
> lldp_tlv_end(p, start);
> }
>
> /* System description (skip it if empty) */
> - if (chassis->c_descr && *chassis->c_descr != '\0') {
> + if (nullable_string_not_empty(chassis->c_descr)) {
> lldp_tlv_start(p, LLDP_TLV_SYSTEM_DESCR, &start);
> dp_packet_put(p, chassis->c_descr, strlen(chassis->c_descr));
> lldp_tlv_end(p, start);
> @@ -231,7 +232,7 @@ lldp_send(struct lldpd *global OVS_UNUSED,
> }
>
> /* Port description */
> - if (port->p_descr && *port->p_descr != '\0') {
> + if (nullable_string_not_empty(port->p_descr)) {
> lldp_tlv_start(p, LLDP_TLV_PORT_DESCR, &start);
> dp_packet_put(p, port->p_descr, strlen(port->p_descr));
> lldp_tlv_end(p, start);
> diff --git a/lib/util.h b/lib/util.h
> index c1fd120bc..b3928badb 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -197,6 +197,12 @@ void free_pagealign(void *);
> OVS_RETURNS_NONNULL void *xmalloc_size_align(size_t, size_t) MALLOC_LIKE;
> void free_size_align(void *);
>
> +static inline bool
> +nullable_string_not_empty(const char *s)
It’s a bit longer, but for consistency, I would include “is” in the function
name, i.e., nullable_string_is_not_empty().
However, I would personally prefer the reverse of this function, i.e.,
str_is_null_or_empty(), possibly in addition to a nullable_strlen() (though
that wouldn’t provide any performance benefit in this case).
> +{
> + return s && *s != '\0';
> +}
> +
> /* The C standards say that neither the 'dst' nor 'src' argument to
> * memcpy() may be null, even if 'n' is zero. This wrapper tolerates
> * the null case. */
> --
> 2.43.5
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
