Re: [Xen-devel] [PATCH 5/9] libxl: Do root checks once in libxl__domain_get_device_model_uid

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

George Dunlap writes ("Re: [PATCH 5/9] libxl: Do root checks once in 
libxl__domain_get_device_model_uid"):
> > On Nov 28, 2018, at 4:39 PM, Ian Jackson <ian.jackson@xxxxxxxxxx> wrote:
> > I know that in the hypervisor this kind of thing is tolerated (wrongly
> > IMO) but can we please not have it here.
> 
> It is a bit strange having to work with one maintianer who thinks a handful 
> of simple gotos is an issue, and another maintainer who thinks having switch 
> case statements appear in the middle of if() { } blocks is perfectly normal. 
> :-)

Sorry, I missed that.

I think maybe indeed a CODING_STYLE update is called for.

> > This may mean splitting stuff out into a sub-function.  That could be
> > done some time between "Move dm user determination logic into a helper
> > function" and this patch I guess.
> 
> I’m afraid you’re going to have to give me a bit more guidance here: It’s not 
> clear to me what would be split into a sub-function, and how that would make 
> the code easier to follow while avoiding unnecessary code duplication.
> 
> Do you propose replacing “goto root_check;” with “root_check(); goto out;” in 
> all locations?  Or something else?

I am afraid that I have not followed all of the strands of code here.
I am very tempted to reply simply with a reference to Dijkstra [1968],
but:

I think some of your gotos could be refactored into early exits from a
new subfunction.  In any case, I would like the code to be made out of
block structured pieces such as for and while loops (perhaps with
early exits like break and continue), if statements, switch
statements, conditionals, and so on.

The `goto out' paradigm (which should be encouraged) is IMO an
exception to this.  But then there should be only one `out' label,
or maybe exceptionally both `out' and `err'.

(I also deprecate the `retry' labels found in some places to deal with
xenstore.)

HTH.

Thanks,
Ian.

References:
  Dijkstra [1968]   _Go To Statement Considered Harmful_
                    March 1968
                    Comm. ACM, 11 (3), 147-148
                    doi:10.1145/362929.362947

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel