LGTM
On Tue, Jul 29, 2025 at 9:02 AM Thomas Huth <th...@redhat.com> wrote:
> On 28/07/2025 19.53, Daniel P. Berrangé wrote:
> > On Mon, Jul 28, 2025 at 07:30:16PM +0200, Thomas Huth wrote:
> >> From: Thomas Huth <th...@redhat.com>
> >>
> >> When compiling QEMU with --enable-ubsan there is a undefined behavior
> >> warning when running "make check":
> >>
> >> .../qga/commands-linux.c:452:15: runtime error: applying non-zero
> offset 5 to null pointer
> >> #0 0x55ea7b89450c in build_guest_fsinfo_for_pci_dev
> ..../qga/commands-linux.c:452:15
> >>
> >> Add a check to avoid incrementing the NULL pointer here.
> >>
> >> Signed-off-by: Thomas Huth <th...@redhat.com>
> >> ---
> >> qga/commands-linux.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
> >> index 9e8a934b9a6..caf7c3ca22b 100644
> >> --- a/qga/commands-linux.c
> >> +++ b/qga/commands-linux.c
> >> @@ -449,7 +449,9 @@ static bool build_guest_fsinfo_for_pci_dev(char
> const *syspath,
> >> has_ata = true;
> >> } else {
> >> p = strstr(syspath, "/host");
> >> - q = p + 5;
> >> + if (p) {
> >> + q = p + 5;
> >> + }
> >> }
> >> if (p && sscanf(q, "%u", &host) == 1) {
> >
> > q is always non-NULL if p is non-NULL, so this is safe, but I would be
> more
> > happy with this changing to 'q && sscanf' to eliminate the indirection.
>
> If we agree to do a bigger change here, I'd rather drop the "q" pointer
> completely and use a new integer variable instead, something like:
>
> int offset;
> ...
> p = strstr(syspath, "/ata");
> if (p) {
> offset = 4;
> has_ata = true;
> } else {
> offset = 5;
> p = strstr(syspath, "/host");
> }
> if (p && sscanf(p + offset, "%u", &host) == 1) {
> ...
> }
>
> WDYT?
>
> Thomas
>
>
