On 8/9/24 17:35, Rosemarie O'Riorden wrote:
> On 8/9/24 10:22 AM, Ilya Maximets wrote:
>> But it might be possible to make it simpler by reducing the amount of nested
>> 'if' statements. For example, we could keep the original check as-is adding
>> the 'continue' after the row deletion and then get the port and perform port
>> checks afterwards without worrying about nb_smb being NULL, i.e.:
>>
>> if (!nb_smb) {
>> sbrec_static_mac_binding_delete(sb_smb);
>> continue;
>> }
>>
>> <Get the port, do other checks and delete the row, if failed>
>>
>> We also may not need to define any extra variables, beside 'op', in this
>> case.
>>
>> What do you think?
>
> I see. That is simpler.
> Like this?:
>
> if (!nb_smb) {
> sbrec_static_mac_binding_delete(sb_smb);
> + continue;
> + }
> + struct ovn_port *op = op = ovn_port_find(lr_ports,
> nb_smb->logical_port);
> + if (!(op && op->nbrp && op->od && op->od->sb)) {
> + sbrec_static_mac_binding_delete(sb_smb);
> }
> }
>
> It builds and passes all checks. I can send a v2.
>
Yeah. Looks good to me. I'd add an empty line before the port lookup
and expand the !(a && b ...) into !a || !b ... . Seems easier to read
this way, but up to you.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
