Re: Re: [PATCHv2 4/4] netlink: Introduce a helper function to simplify netlink functions

On 2021-01-06 at 18:21, Michal Privoznik wrote: >On 1/6/21 3:40 AM, Shi Lei wrote: >> Extract common code as helper function virNetlinkTalk, then simplify >> the functions virNetlink[DumpLink|NewLink|DelLink|GetNeighbor]. >> >> -    default: >> -        goto malformed_resp; >> +    if (resp->nlmsg_type != NLMSG_DONE) { >> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> +                       _("malformed netlink response message")); >> +        return -1; >>       } > >This is not correct IMO. The way that control flows through this >function before your patch is (I'm trying to create virbr0 using 'virsh >net-start default'): > >1) virNetlinkCommand() suceeds, >2) resp->nlmsg_type == NLMSG_ERROR even though the bridge was created, >3) err->error is 0 hence we hit 'break' and fall out of the switch and .. > >>   >>       return 0; Yes. So the result ((resp->nlmsg_type == NLMSG_ERROR) && (err->error == 0)) should be regarded as a success. I feel like that this piece of code can be like:     if (((resp->nlmsg_type != NLMSG_ERROR) || (*error != 0)) &&         (resp->nlmsg_type != NLMSG_DONE) {         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",                                 _("malformed netlink response message"));         return -1;     }     return 0;