On 6/9/2021 8:15 PM, Daniel P. Berrangé wrote:
> On Wed, Jun 09, 2021 at 02:09:47PM +0200, Markus Armbruster wrote:
>> Paolo Bonzini <pbonz...@redhat.com> writes:
>>
>>> On 10/06/21 10:47, Zhenzhong Duan wrote:
>>>> Based on the description of error_setg(), the local variable err in
>>>> qemu_maybe_daemonize() should be initialized to NULL.
>>>> Without fix, the uninitialized *errp triggers assert failure which
>>>> doesn't show much valuable information.
>>>> Before the fix:
>>>> qemu-system-x86_64: ../util/error.c:59: error_setv: Assertion `*errp ==
>>>> NULL' failed.
>>>> After fix:
>>>> qemu-system-x86_64: cannot create PID file: Cannot open pid file:
>>>> Permission denied
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>>>> ---
>>>> softmmu/vl.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>>>> index 326c1e9080..feb4d201f3 100644
>>>> --- a/softmmu/vl.c
>>>> +++ b/softmmu/vl.c
>>>> @@ -2522,7 +2522,7 @@ static void qemu_process_help_options(void)
>>>> static void qemu_maybe_daemonize(const char *pid_file)
>>>> {
>>>> - Error *err;
>>>> + Error *err = NULL;
>>
>> Common mistake, I'm afraid.
>
> Initializing isn't likely to be a performance impact, so I'd think
> we should make 'checkpatch.pl' complain about any 'Error *' variable
> that is not initialized to NULL, as a safety net, even if not technically
> required in some cases.
>
> Regards,
> Daniel
>
Hi,
Could we add a coccinelle script to check (and fix) these problems? e.g.:
@ r @
identifier id;
@@
Error *id
+ = NULL
;
Using this script, I found that local variable err in
qemu_init_subsystems is not initialized to NULL too. The script is not
prefect though, it will initialize all global/static 'Error *' variables
and all local 'Error *' variables in util/error.c to NULL, which is
unnecessary.
Thanks,
Peng
