[PATCH] softmmu/physmem.c: Remove unneeded NULL check in qemu_ram_alloc_from_fd()

Peter Maydell posted 1 patch 2 years, 8 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210812150624.29139-1-peter.maydell@linaro.org
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, David Hildenbrand <david@redhat.com>, Peter Xu <peterx@redhat.com>
softmmu/physmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] softmmu/physmem.c: Remove unneeded NULL check in qemu_ram_alloc_from_fd()
Posted by Peter Maydell 2 years, 8 months ago
In the alignment check added to qemu_ram_alloc_from_fd() in commit
ce317be98db0dfdfa, the condition includes a check that 'mr' is not
NULL.  This check is unnecessary because we can assume that the
caller always passes us a valid MemoryRegion, and indeed later in the
function we assume mr is not NULL when we pass it to file_ram_alloc()
as new_block->mr.  Remove it.

Fixes: Coverity 1459867
Fixes: ce317be98d ("exec: fetch the alignment of Linux devdax pmem character device nodes")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 softmmu/physmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 3c1912a1a07..c47cb6da2e4 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2075,7 +2075,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
     }
 
     file_align = get_file_align(fd);
-    if (file_align > 0 && mr && file_align > mr->align) {
+    if (file_align > 0 && file_align > mr->align) {
         error_setg(errp, "backing store align 0x%" PRIx64
                    " is larger than 'align' option 0x%" PRIx64,
                    file_align, mr->align);
-- 
2.20.1


Re: [PATCH] softmmu/physmem.c: Remove unneeded NULL check in qemu_ram_alloc_from_fd()
Posted by Liu, Jingqi 2 years, 8 months ago

On 8/12/2021 11:06 PM, Peter Maydell wrote:
> In the alignment check added to qemu_ram_alloc_from_fd() in commit
> ce317be98db0dfdfa, the condition includes a check that 'mr' is not
> NULL.  This check is unnecessary because we can assume that the
> caller always passes us a valid MemoryRegion, and indeed later in the
> function we assume mr is not NULL when we pass it to file_ram_alloc()
> as new_block->mr.  Remove it.
> 
> Fixes: Coverity 1459867
> Fixes: ce317be98d ("exec: fetch the alignment of Linux devdax pmem character device nodes")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   softmmu/physmem.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 3c1912a1a07..c47cb6da2e4 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -2075,7 +2075,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
>       }
>   
>       file_align = get_file_align(fd);
> -    if (file_align > 0 && mr && file_align > mr->align) {
> +    if (file_align > 0 && file_align > mr->align) {
>           error_setg(errp, "backing store align 0x%" PRIx64
>                      " is larger than 'align' option 0x%" PRIx64,
>                      file_align, mr->align);
> 
'mr' is indeed not NULL in the current code.

Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>

Thanks,
Jingqi