| Message ID | 20170322204844.446-4-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Philippe Mathieu-Daudé (2017-03-22 15:48:44) > static code analyzer complain: > > qga/commands-posix.c:2127:9: warning: Null pointer passed as an argument to a 'nonnull' parameter > closedir(dp); > ^~~~~~~~~~~~ > > Reported-by: Clang Static Analyzer > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > qga/commands-posix.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 73d93eb5ce..8028141534 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -2122,9 +2122,11 @@ static void transfer_memory_block(GuestMemoryBlock *mem_blk, bool sys2memblk, > * we think this VM does not support online/offline memory block, > * any other solution? > */ > - if (!dp && errno == ENOENT) { > - result->response = > - GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED; > + if (!dp) { > + if (errno == ENOENT) { > + result->response = > + GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED; > + } > goto out1; > } I think this should be: if (!dp) { if (errno == ENOENT) { result->response = GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED; } else { result->response = GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_FAILED; } goto out1; } otherwise we might set result->error_code while still indicating success for the operation. That wasn't handled properly before your patch either, it's just more apparent now. > closedir(dp); > -- > 2.11.0 >
Hi Michael, On 03/22/2017 06:22 PM, Michael Roth wrote: > Quoting Philippe Mathieu-Daudé (2017-03-22 15:48:44) >> static code analyzer complain: >> >> qga/commands-posix.c:2127:9: warning: Null pointer passed as an argument to a 'nonnull' parameter >> closedir(dp); >> ^~~~~~~~~~~~ >> >> Reported-by: Clang Static Analyzer >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> --- >> qga/commands-posix.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >> index 73d93eb5ce..8028141534 100644 >> --- a/qga/commands-posix.c >> +++ b/qga/commands-posix.c >> @@ -2122,9 +2122,11 @@ static void transfer_memory_block(GuestMemoryBlock *mem_blk, bool sys2memblk, >> * we think this VM does not support online/offline memory block, >> * any other solution? >> */ >> - if (!dp && errno == ENOENT) { >> - result->response = >> - GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED; >> + if (!dp) { >> + if (errno == ENOENT) { >> + result->response = >> + GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED; >> + } >> goto out1; >> } > > I think this should be: > > if (!dp) { > if (errno == ENOENT) { > result->response = > GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED; > } else { > result->response = > GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_FAILED; > } > goto out1; > } > > otherwise we might set result->error_code while still indicating > success for the operation. That wasn't handled properly before your > patch either, it's just more apparent now. Indeed, I'll resend it in 2 patches It seems easier to me to understand. > >> closedir(dp); >> -- >> 2.11.0 >> > >
diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 73d93eb5ce..8028141534 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -2122,9 +2122,11 @@ static void transfer_memory_block(GuestMemoryBlock *mem_blk, bool sys2memblk, * we think this VM does not support online/offline memory block, * any other solution? */ - if (!dp && errno == ENOENT) { - result->response = - GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED; + if (!dp) { + if (errno == ENOENT) { + result->response = + GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED; + } goto out1; } closedir(dp);
static code analyzer complain: qga/commands-posix.c:2127:9: warning: Null pointer passed as an argument to a 'nonnull' parameter closedir(dp); ^~~~~~~~~~~~ Reported-by: Clang Static Analyzer Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- qga/commands-posix.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)