| Message ID | 240e4b9059fd0c3afced27edf7a96b7842b39777.1760112598.git.echaudro@redhat.com |
|---|---|
| State | Accepted |
| Commit | 1577bfe40437b0dac900e573acd2b75670210dc8 |
| Delegated to: | Eelco Chaudron |
| Headers | show |
| Series | [ovs-dev] netdev-vport: Free error string from str_to_u8() in tunnel config. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/cirrus-robot | success | cirrus build: passed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 10/10/2025 17:09, Eelco Chaudron via dev wrote: > Free the error string returned by str_to_u8() in set_tunnel_config(). > While at it, also add a proper error message for this error case. > > Fixes: ebe0e518b048 ("tunnel: Bareudp Tunnel Support.") > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > --- > lib/netdev-vport.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c > index ed67b509d..d11269d00 100644 > --- a/lib/netdev-vport.c > +++ b/lib/netdev-vport.c > @@ -813,9 +813,13 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args, char **errp) > tnl_cfg.exts |= (1 << OVS_BAREUDP_EXT_MULTIPROTO_MODE); > } else { > uint16_t payload_ethertype; > + char *error = str_to_u16(node->value, "payload_type", > + &payload_ethertype); Alternative would be to use str_to_uint() directly, but there'd be a bit of messing around with bounds checks and types, so it's probably more readable as you have it. Acked-by: Kevin Traynor <ktraynor@redhat.com> > > - if (str_to_u16(node->value, "payload_type", > - &payload_ethertype)) { > + if (error) { > + free(error); > + ds_put_format(&errors, "%s: bad %s 'payload_type'\n", > + name, node->value); > err = EINVAL; > goto out; > }
On 14 Oct 2025, at 12:08, Kevin Traynor wrote: > On 10/10/2025 17:09, Eelco Chaudron via dev wrote: >> Free the error string returned by str_to_u8() in set_tunnel_config(). >> While at it, also add a proper error message for this error case. >> >> Fixes: ebe0e518b048 ("tunnel: Bareudp Tunnel Support.") >> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> >> --- >> lib/netdev-vport.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >> index ed67b509d..d11269d00 100644 >> --- a/lib/netdev-vport.c >> +++ b/lib/netdev-vport.c >> @@ -813,9 +813,13 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args, char **errp) >> tnl_cfg.exts |= (1 << OVS_BAREUDP_EXT_MULTIPROTO_MODE); >> } else { >> uint16_t payload_ethertype; >> + char *error = str_to_u16(node->value, "payload_type", >> + &payload_ethertype); > > Alternative would be to use str_to_uint() directly, but there'd be a bit > of messing around with bounds checks and types, so it's probably more > readable as you have it. > Acked-by: Kevin Traynor <ktraynor@redhat.com> Thanks, Kevin. Yeah, I was also a bit confused by the various str_to_xx() helpers, some return a boolean, others an error string, so I figured this approach would be the simplest and most consistent. Thanks for the ack! //Eelco >> - if (str_to_u16(node->value, "payload_type", >> - &payload_ethertype)) { >> + if (error) { >> + free(error); >> + ds_put_format(&errors, "%s: bad %s 'payload_type'\n", >> + name, node->value); >> err = EINVAL; >> goto out; >> }
On 14 Oct 2025, at 12:08, Kevin Traynor wrote: > On 10/10/2025 17:09, Eelco Chaudron via dev wrote: >> Free the error string returned by str_to_u8() in set_tunnel_config(). >> While at it, also add a proper error message for this error case. >> >> Fixes: ebe0e518b048 ("tunnel: Bareudp Tunnel Support.") >> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Thanks, Kevin, for the review, applied to main and backported all the way to 3.3. //Eelco
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index ed67b509d..d11269d00 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -813,9 +813,13 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args, char **errp) tnl_cfg.exts |= (1 << OVS_BAREUDP_EXT_MULTIPROTO_MODE); } else { uint16_t payload_ethertype; + char *error = str_to_u16(node->value, "payload_type", + &payload_ethertype); - if (str_to_u16(node->value, "payload_type", - &payload_ethertype)) { + if (error) { + free(error); + ds_put_format(&errors, "%s: bad %s 'payload_type'\n", + name, node->value); err = EINVAL; goto out; }
Free the error string returned by str_to_u8() in set_tunnel_config(). While at it, also add a proper error message for this error case. Fixes: ebe0e518b048 ("tunnel: Bareudp Tunnel Support.") Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- lib/netdev-vport.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)