Re: [PATCH v2 1/2] public/io: xs_wire: Document that EINVAL should always be first in xsd_errors

[Juergen Gross <jgross@xxxxxxxx>]

On 27.06.22 16:48, Julien Grall wrote:
> Hi,
>
> On 27/06/2022 15:31, Juergen Gross wrote:
> > On 27.06.22 14:36, Julien Grall wrote:
> > > From: Julien Grall <jgrall@xxxxxxxxxx>
> > >
> > > Some tools (e.g. xenstored) always expect EINVAL to be first in xsd_errors.
> > >
> > > Document it so, one doesn't add a new entry before hand by mistake.
> > >
> > > Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> > >
> > > ----
> > >
> > > I have tried to add a BUILD_BUG_ON() but GCC complained that the value
> > > was not a constant. I couldn't figure out a way to make GCC happy.
> > >
> > > Changes in v2:
> > >      - New patch
> > > ---
> > >   xen/include/public/io/xs_wire.h | 1 +
> > >   1 file changed, 1 insertion(+)
> > >
> > > diff --git a/xen/include/public/io/xs_wire.h b/xen/include/public/io/xs_wire.h
> > > index c1ec7c73e3b1..dd4c9c9b972d 100644
> > > --- a/xen/include/public/io/xs_wire.h
> > > +++ b/xen/include/public/io/xs_wire.h
> > > @@ -76,6 +76,7 @@ static struct xsd_errors xsd_errors[]
> > >   __attribute__((unused))
> > >   #endif
> > >       = {
> > > +    /* /!\ Some users (e.g. xenstored) expect EINVAL to be the first entry. */
> > >       XSD_ERROR(EINVAL),
> > >       XSD_ERROR(EACCES),
> > >       XSD_ERROR(EEXIST),
> > What about another approach, like:
> In place of what? I still think we need the comment because this assumption is 
> not part of the ABI (AFAICT xs_wire.h is meant to be stable).
> At which point, I see limited reason to fix xenstored_core.c.
>
> But I would have really prefer to use a BUILD_BUG_ON() (or similar) so we can 
> catch any misue a build. Maybe I should write a small program that is executed 
> at compile time?
My suggestion removes the need for EINVAL being the first entry.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature