Re: [Xen-devel] [PATCH v5 06/10] arm: smccc: handle SMCs according to SMCCC

[Julien Grall <julien.grall@xxxxxxx>]

On 19/09/17 22:44, Volodymyr Babchuk wrote:
> Hi Julien,
Hi Volodymyr,

> On 13.09.17 14:11, Julien Grall wrote:
> > Hi,
> >
> > On 08/31/2017 09:09 PM, Volodymyr Babchuk wrote:
> > > +static void fill_uuid(struct cpu_user_regs *regs, const xen_uuid_t *u)
> > Actually why do you pass a pointer for u? This requires every caller 
> > to introduce temporary variable because the UUID is usually a define.
> Hmm, another way probably is to pass a whole structure as a parameter.
> Are you suggesting this approach? Something like
> fill_uuid(regs, (xen_uuid_t)MY_UUID)?
Something list that. But why do you need the cast? MY_UUID is supposed 
to be a xen_uuid_t. No?
> > With your current solution each caller as to do:
> >
> > xen_uuid_t foo = MY_UUID;
> >
> > fill_uuid(regs, &foo);
> >
> > return true;
> >
> > What I suggested in the previous version is to get fill_uuid return 
> > true. So you make each caller simpler.
> Yes, but it will not be correct semantically. There will arise many 
> questions:
> 1. Why helper function that only writes data returns bool?
> 2. If it returns true, can it return false?
> 3. Should we check its return value before passing it further?

I really don't see how

return fill_uuid(regs, MY_UUID);

would be semantically incorrect or even raise all those questions. It is 
perfectly fine to always return true. We have place like that and it 
helps to streamline the code.
Cheers,

--
Julien Grall

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