On 07/13/20 15:13, Peter Maydell wrote:
> On Sat, 4 Jul 2020 at 17:41, Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
>>
>> The 'gen_id' argument refers to a QOM object able to produce
>> data consumable by the fw_cfg device. The producer object must
>> implement the FW_CFG_DATA_GENERATOR interface.
>>
>> Reviewed-by: Laszlo Ersek <ler...@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>> Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
>> Message-Id: <20200623172726.21040-4-phi...@redhat.com>
>
> Coverity points out (CID 1430396) an issue with the error handling
> in this patch:
>
>
>> @@ -2052,6 +2056,15 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts,
>> Error **errp)
>> if (nonempty_str(str)) {
>> size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
>> buf = g_memdup(str, size);
>> + } else if (nonempty_str(gen_id)) {
>> + Error *local_err = NULL;
>
> We set local_err to NULL here...
>
>> +
>> + fw_cfg_add_from_generator(fw_cfg, name, gen_id, errp);
>
> ...but we don't pass it to the function here...
Ugh, I should have noticed that in review. I'm sorry.
Laszlo
>
>> + if (local_err) {
>
> ...so this condition is always false and the body of the if is dead code.
>
>> + error_propagate(errp, local_err);
>> + return -1;
>> + }
>> + return 0;
>> } else {
>> GError *err = NULL;
>> if (!g_file_get_contents(file, &buf, &size, &err)) {
>
> thanks
> -- PMM
>
