[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT/NEWLIB PATCH 2/2] add declarations needed for vfscore in the gluecode



Hi Yuri,

On 3/5/19 6:26 PM, Yuri Volchkov wrote:
diff --git a/include/sys/stat.h b/include/sys/stat.h
new file mode 100644
index 0000000..21a3f78
--- /dev/null
+++ b/include/sys/stat.h
@@ -0,0 +1,11 @@
+#ifndef _NEWLIB_GLUE_SYS_STAT_H_
+#define _NEWLIB_GLUE_SYS_STAT_H_
+
+#define __rtems__
+#include_next <sys/stat.h>
+#undef __rtems__

Would it make sense to check first whether __rtems__ is defined, and
only redefine and undefine it again in that case? Because after
including that header files, it is now unconditionally undefined, which
might not be what is expected?
I am going just to draw attention if __rtems__ is defined beforehand for
2 reasons:

1) I am not sure I understand what stands behind this __rtems__ in
    newlib, but it breaks the build pretty badly. If someone comes here
    later with an intention to enable __rtems__ globaly, I would assume
    he/she will know better what is it for. And that would make sense to
    reconsider this wrapper.

2) The construction for defining and undefining __rtems__ looks super
    ugly:
       #ifndef __rtems__
       #define __rtems__
       #include_next <sys/stat.h>
       #undef __rtems__
       #else
       #include_next <sys/stat.h>
       #endif

I think 2) is not a good reason, because ugly is still better than wrong. ;-) However, I think 1) makes sense. RTEMS is a real-time operating system, so __rtems__ should in general only be set if you compile newlib for RTEMS. Considering unikraft isn't RTEMS, it seems to make sense to me that we can expect that this isn't set.

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.