Re: [Xen-devel] [PATCH v2 2/2] libxl/devd: correctly manipulate the dguest list

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[PATCH v2 2/2] libxl/devd: correctly manipulate the 
dguest list"):
> Current code in backend_watch_callback has two issues when manipulating the
> dguest list:
...
>  skip:
>      libxl__nested_ao_free(nested_ao);
> +clean:
>      if (ddev)
>          free(ddev->dev);

This is starting to be quite goto-rich, and the memory ownership rules
become less clear.  Rather than try to analyse this in detail, I
wonder if it would be better to try to rework this so that it fits
CODING_STYLE better.

Wei, what do you think ?

>      free(ddev);
> -    free(dguest);
> +    if (dguest != NULL &&
> +        dguest->num_vifs + dguest->num_vbds + dguest->num_qdisks == 0) {

Perhaps this cleanup functionality could become a function if its own.
check_maybe_free_dguest or something ?

I haven't gone through the code in detail trying to convince myself
it's OK.  Even if there are to be no significant code changes, I would
like to see the memory ownership and lifetime rules here written down
(in comments in the code).  That way readers wouldn't have to
reverse-engineer them, and bugs will be clearer.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel