sscanf return values are checked and add 'Null' check for
mandatory parameters. And merged redundant route and
networkroute variables.
Signed-off-by: Dehan Meng <demeng@redhat.com>
---
qga/commands-linux.c | 86 +++++++++++++++++++++++---------------------
1 file changed, 45 insertions(+), 41 deletions(-)
diff --git a/qga/commands-linux.c b/qga/commands-linux.c
index 51d5e3d927..495de74b31 100644
--- a/qga/commands-linux.c
+++ b/qga/commands-linux.c
@@ -2103,7 +2103,9 @@ static char *hexToIPAddress(const void *hexValue, int is_ipv6)
int i;
for (i = 0; i < 16; i++) {
- sscanf(&hexStr[i * 2], "%02hhx", &in6.s6_addr[i]);
+ if (sscanf(&hex_str[i * 2], "%02hhx", &in6.s6_addr[i]) != 1) {
+ return NULL;
+ }
}
inet_ntop(AF_INET6, &in6, addr, INET6_ADDRSTRLEN);
@@ -2160,26 +2162,25 @@ GuestNetworkRouteList *qmp_guest_network_get_route(Error **errp)
continue;
}
- route = g_new0(GuestNetworkRoute, 1);
- networkroute = route;
- networkroute->iface = g_strdup(Iface);
- networkroute->destination = hexToIPAddress(Destination, 1);
- networkroute->metric = Metric;
- networkroute->source = hexToIPAddress(Source, 1);
- networkroute->desprefixlen = g_strdup_printf(
- "%d", DesPrefixlen
- );
- networkroute->srcprefixlen = g_strdup_printf(
- "%d", SrcPrefixlen
- );
- networkroute->nexthop = hexToIPAddress(NextHop, 1);
- networkroute->has_flags = true;
- networkroute->flags = Flags;
- networkroute->has_refcnt = true;
- networkroute->refcnt = RefCnt;
- networkroute->has_use = true;
- networkroute->use = Use;
- networkroute->version = 6;
+ g_autoptr(GuestNetworkRoute) route = g_new0(GuestNetworkRoute, 1);
+
+ route->destination = hex_to_ip_address(destination, 1);
+ route->iface = g_strdup(iface);
+ if (route->destination == NULL) {
+ continue;
+ }
+ route->source = hex_to_ip_address(source, 1);
+ route->nexthop = hex_to_ip_address(next_hop, 1);
+ route->desprefixlen = g_strdup_printf("%d", des_prefixlen);
+ route->srcprefixlen = g_strdup_printf("%d", src_prefixlen);
+ route->metric = metric;
+ route->has_flags = true;
+ route->flags = flags;
+ route->has_refcnt = true;
+ route->refcnt = refcnt;
+ route->has_use = true;
+ route->use = use;
+ route->version = 6;
} else {
unsigned int Destination, Gateway, Mask, Flags;
int RefCnt, Use, Metric, MTU, Window, IRTT;
@@ -2191,26 +2192,29 @@ GuestNetworkRouteList *qmp_guest_network_get_route(Error **errp)
continue;
}
- route = g_new0(GuestNetworkRoute, 1);
- networkroute = route;
- networkroute->iface = g_strdup(Iface);
- networkroute->destination = hexToIPAddress(&Destination, 0);
- networkroute->gateway = hexToIPAddress(&Gateway, 0);
- networkroute->mask = hexToIPAddress(&Mask, 0);
- networkroute->metric = Metric;
- networkroute->has_flags = true;
- networkroute->flags = Flags;
- networkroute->has_refcnt = true;
- networkroute->refcnt = RefCnt;
- networkroute->has_use = true;
- networkroute->use = Use;
- networkroute->has_mtu = true;
- networkroute->mtu = MTU;
- networkroute->has_window = true;
- networkroute->window = Window;
- networkroute->has_irtt = true;
- networkroute->irtt = IRTT;
- networkroute->version = 4;
+ g_autoptr(GuestNetworkRoute) route = g_new0(GuestNetworkRoute, 1);
+
+ route->destination = hex_to_ip_address(destination, 1);
+ route->iface = g_strdup(iface);
+ if (route->destination == NULL) {
+ continue;
+ }
+ route->gateway = hex_to_ip_address(&gateway, 0);
+ route->mask = hex_to_ip_address(&mask, 0);
+ route->metric = metric;
+ route->has_flags = true;
+ route->flags = flags;
+ route->has_refcnt = true;
+ route->refcnt = refcnt;
+ route->has_use = true;
+ route->use = use;
+ route->has_mtu = true;
+ route->mtu = mtu;
+ route->has_window = true;
+ route->window = window;
+ route->has_irtt = true;
+ route->irtt = irtt;
+ route->version = 4;
}
QAPI_LIST_APPEND(tail, route);
--
2.40.1Best Regards,
Konstantin Kostiuk.
On Wed, Nov 6, 2024 at 10:41 AM Dehan Meng <demeng@redhat.com> wrote:
> sscanf return values are checked and add 'Null' check for
> mandatory parameters. And merged redundant route and
> networkroute variables.
>
> Signed-off-by: Dehan Meng <demeng@redhat.com>
> ---
> qga/commands-linux.c | 86 +++++++++++++++++++++++---------------------
> 1 file changed, 45 insertions(+), 41 deletions(-)
>
> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
> index 51d5e3d927..495de74b31 100644
> --- a/qga/commands-linux.c
> +++ b/qga/commands-linux.c
> @@ -2103,7 +2103,9 @@ static char *hexToIPAddress(const void *hexValue,
> int is_ipv6)
> int i;
>
> for (i = 0; i < 16; i++) {
> - sscanf(&hexStr[i * 2], "%02hhx", &in6.s6_addr[i]);
> + if (sscanf(&hex_str[i * 2], "%02hhx", &in6.s6_addr[i]) != 1) {
> + return NULL;
> + }
> }
> inet_ntop(AF_INET6, &in6, addr, INET6_ADDRSTRLEN);
>
> @@ -2160,26 +2162,25 @@ GuestNetworkRouteList
> *qmp_guest_network_get_route(Error **errp)
> continue;
> }
>
> - route = g_new0(GuestNetworkRoute, 1);
> - networkroute = route;
> - networkroute->iface = g_strdup(Iface);
> - networkroute->destination = hexToIPAddress(Destination,
> 1);
> - networkroute->metric = Metric;
> - networkroute->source = hexToIPAddress(Source, 1);
> - networkroute->desprefixlen = g_strdup_printf(
> - "%d", DesPrefixlen
> - );
> - networkroute->srcprefixlen = g_strdup_printf(
> - "%d", SrcPrefixlen
> - );
> - networkroute->nexthop = hexToIPAddress(NextHop, 1);
> - networkroute->has_flags = true;
> - networkroute->flags = Flags;
> - networkroute->has_refcnt = true;
> - networkroute->refcnt = RefCnt;
> - networkroute->has_use = true;
> - networkroute->use = Use;
> - networkroute->version = 6;
> + g_autoptr(GuestNetworkRoute) route =
> g_new0(GuestNetworkRoute, 1);
> +
> + route->destination = hex_to_ip_address(destination, 1);
>
Compilation failed
../qga/commands-linux.c: In function ‘qmp_guest_network_get_route’:
../qga/commands-linux.c:2189:56: error: passing argument 1 of
‘hex_to_ip_address’ makes pointer from integer without a cast
[-Wint-conversion]
2189 | route->destination = hex_to_ip_address(destination,
1);
| ^~~~~~~~~~~
| |
| unsigned int
../qga/commands-linux.c:2096:44: note: expected ‘const void *’ but argument
is of type ‘unsigned int’
2096 | static char *hex_to_ip_address(const void *hex_value, int is_ipv6)
| ~~~~~~~~~~~~^~~~~~~~~
You use `destination` but the variable declaration was not updated from
`Destination`.
The same for DesPrefixlen/des_prefixlen, Flags/flags, etc.
Looks like you mixed changes from the next commit into this one.
+ route->iface = g_strdup(iface);
> + if (route->destination == NULL) {
> + continue;
> + }
> + route->source = hex_to_ip_address(source, 1);
> + route->nexthop = hex_to_ip_address(next_hop, 1);
> + route->desprefixlen = g_strdup_printf("%d",
> des_prefixlen);
> + route->srcprefixlen = g_strdup_printf("%d",
> src_prefixlen);
> + route->metric = metric;
> + route->has_flags = true;
> + route->flags = flags;
> + route->has_refcnt = true;
> + route->refcnt = refcnt;
> + route->has_use = true;
> + route->use = use;
> + route->version = 6;
> } else {
> unsigned int Destination, Gateway, Mask, Flags;
> int RefCnt, Use, Metric, MTU, Window, IRTT;
> @@ -2191,26 +2192,29 @@ GuestNetworkRouteList
> *qmp_guest_network_get_route(Error **errp)
> continue;
> }
>
> - route = g_new0(GuestNetworkRoute, 1);
> - networkroute = route;
> - networkroute->iface = g_strdup(Iface);
> - networkroute->destination = hexToIPAddress(&Destination,
> 0);
> - networkroute->gateway = hexToIPAddress(&Gateway, 0);
> - networkroute->mask = hexToIPAddress(&Mask, 0);
> - networkroute->metric = Metric;
> - networkroute->has_flags = true;
> - networkroute->flags = Flags;
> - networkroute->has_refcnt = true;
> - networkroute->refcnt = RefCnt;
> - networkroute->has_use = true;
> - networkroute->use = Use;
> - networkroute->has_mtu = true;
> - networkroute->mtu = MTU;
> - networkroute->has_window = true;
> - networkroute->window = Window;
> - networkroute->has_irtt = true;
> - networkroute->irtt = IRTT;
> - networkroute->version = 4;
> + g_autoptr(GuestNetworkRoute) route =
> g_new0(GuestNetworkRoute, 1);
> +
> + route->destination = hex_to_ip_address(destination, 1);
> + route->iface = g_strdup(iface);
> + if (route->destination == NULL) {
> + continue;
> + }
> + route->gateway = hex_to_ip_address(&gateway, 0);
> + route->mask = hex_to_ip_address(&mask, 0);
> + route->metric = metric;
> + route->has_flags = true;
> + route->flags = flags;
> + route->has_refcnt = true;
> + route->refcnt = refcnt;
> + route->has_use = true;
> + route->use = use;
> + route->has_mtu = true;
> + route->mtu = mtu;
> + route->has_window = true;
> + route->window = window;
> + route->has_irtt = true;
> + route->irtt = irtt;
> + route->version = 4;
> }
>
> QAPI_LIST_APPEND(tail, route);
> --
> 2.40.1
>
>
On Wed, Nov 06, 2024 at 04:41:32PM +0800, Dehan Meng wrote: > sscanf return values are checked and add 'Null' check for > mandatory parameters. And merged redundant route and > networkroute variables. > > Signed-off-by: Dehan Meng <demeng@redhat.com> > --- > qga/commands-linux.c | 86 +++++++++++++++++++++++--------------------- > 1 file changed, 45 insertions(+), 41 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com> On Wed, Nov 6, 2024 at 3:35 PM Daniel P. Berrangé <berrange@redhat.com> wrote: > On Wed, Nov 06, 2024 at 04:41:32PM +0800, Dehan Meng wrote: > > sscanf return values are checked and add 'Null' check for > > mandatory parameters. And merged redundant route and > > networkroute variables. > > > > Signed-off-by: Dehan Meng <demeng@redhat.com> > > --- > > qga/commands-linux.c | 86 +++++++++++++++++++++++--------------------- > > 1 file changed, 45 insertions(+), 41 deletions(-) > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > > With regards, > Daniel > -- > |: https://berrange.com -o- > https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- > https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- > https://www.instagram.com/dberrange :| > >
© 2016 - 2024 Red Hat, Inc.