| Message ID | 20241106084134.1133061-2-demeng@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | qemu-ga: Summarize and merge commits | expand |
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
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 :| > >
Best 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 > >
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);
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(-)