On 7/7/20 3:14 AM, Richard Henderson wrote:
> On 6/29/20 12:08 AM, Philippe Mathieu-Daudé wrote:
>> Coverity noticed commit 950c4e6c94 introduced a dereference before
>> null check in get_opt_value (CID1391003):
>>
>> In get_opt_value: All paths that lead to this null pointer
>> comparison already dereference the pointer earlier (CWE-476)
>>
>> We fixed this in commit 6e3ad3f0e31, but relaxed the check in commit
>> 0c2f6e7ee99 because "No callers of get_opt_value() pass in a NULL
>> for the 'value' parameter".
>>
>> Since this function is publicly exposed, it risks new users to do
>> the same error again. Avoid that documenting the 'value' argument
>> must not be NULL.
>
> I think we should also add some use of __attribute__((nonnull(...))) to
> enforce
> this within the compiler.
>
> I recently did this without a qemu/compiler.h QEMU_FOO wrapper within
> target/arm. But the nonnull option has optional arguments, so it might be
> difficult to wrap in macros.
I have this patch after your suggestion from last year:
+#if __has_attribute(nonnull)
+# define QEMU_NONNULL(LIST) __attribute__((nonnull((LIST))))
+#else
+# define QEMU_NONNULL(LIST)
+#endif
Examples:
SpaprDrc *spapr_dr_connector_new(Object *owner, const char *type,
- uint32_t id);
+ uint32_t id) QEMU_NONNULL(1);
SpaprDrc *spapr_drc_by_index(uint32_t index);
SpaprDrc *spapr_drc_by_id(const char *type, uint32_t id);
-int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t
drc_type_mask);
+int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t
drc_type_mask)
+ QEMU_NONNULL(3);
...
/**
* memory_region_init_iommu: Initialize a memory region of a custom type
@@ -1066,7 +1073,8 @@ void memory_region_init_iommu(void *_iommu_mr,
const char *mrtypename,
Object *owner,
const char *name,
- uint64_t size);
+ uint64_t size)
+ QEMU_NONNULL(4);
/**
* memory_region_init_ram - Initialize RAM memory region. Accesses
into the
@@ -1154,7 +1162,8 @@ void memory_region_init_rom_device(MemoryRegion *mr,
void *opaque,
const char *name,
uint64_t size,
- Error **errp);
+ Error **errp)
+ QEMU_NONNULL(2);
I can send as RFC is that looks OK to you.
Regards,
Phil.
