Hi Juergen,
On 27/06/2022 15:50, Juergen Gross wrote:
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 <jgr...@amazon.com>
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 <jgr...@amazon.com>
----
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
xsd_errors[] is part of the stable ABI. If Xenstored is already
"misusing" it, then I wouldn't be surprised if other software rely on
this as well.
Therefore, I don't really see how fixing Xenstored would allow us to
remove this restriction.
The only reason this was spotted is by Jan reviewing C Xenstored.
Without that, it would have problably took a long time to notice
this change (I don't think there are many other errno used by Xenstored
and xsd_errors). So I think the risk is not worth the effort.
At least, this is not a patch I would be willing to have my name on
(either as a signed-off-by or acked-by).
Cheers,
--
Julien Grall
