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

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

Hi Florian,

Florian Schmidt <Florian.Schmidt@xxxxxxxxx> writes:

> Hi Yuri,
>
> a few small comments:
>
> On 3/4/19 6:54 PM, Yuri Volchkov wrote:
>> Signed-off-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
>> ---
>>   include/fcntl.h     | 36 ++++++++++++++++++++++++++++++++++++
>>   include/sys/param.h |  8 ++++++++
>>   include/sys/stat.h  | 11 +++++++++++
>>   include/sys/uio.h   |  2 ++
>>   4 files changed, 57 insertions(+)
>>   create mode 100644 include/fcntl.h
>>   create mode 100644 include/sys/param.h
>>   create mode 100644 include/sys/stat.h
>> 
>> diff --git a/include/fcntl.h b/include/fcntl.h
>> new file mode 100644
>> index 0000000..a880ba2
>> --- /dev/null
>> +++ b/include/fcntl.h
>> @@ -0,0 +1,36 @@
>> +#ifndef _NEWLIB_GLUE_FCNTL_H_
>> +#define _NEWLIB_GLUE_FCNTL_H_
>> +
>> +#include_next <fcntl.h>
>> +#define loff_t off_t
>> +
>> +#include <uk/config.h>
>> +
>> +#if (defined CONFIG_ARCH_X86_64)
>> +#define O_NOFOLLOW  0400000
>> +#define O_DIRECTORY 0200000
>> +#define O_CLOEXEC  02000000
>> +#define O_DSYNC      010000
>> +#endif
>> +
>> +#if ((defined CONFIG_ARCH_ARM_64) || (defined CONFIG_ARCH_ARM_32))
>> +#define O_NOFOLLOW  0100000
>> +#define O_DIRECTORY  040000
>> +#define O_CLOEXEC  02000000
>> +#define O_DSYNC      010000
>> +#endif
>> +
>> +
>> +/* Glibc does not provide KEEP_SIZE and PUNCH_HOLE anymore. Instead it
>> + * includes * linux/falloc.h.
>> + *
>> + * Musl still does provide them. And Newlib is just Newlib
>> + */
>
> There's an extra asterisk on line 2 of the comment. And Newlib is just 
> Newlib, eh? ;) That sentence shuld have at least one period at the end. 
> Or maybe three, if you think that fits better...
Oops, formatting mistake

>
>> +#define FALLOC_FL_KEEP_SIZE 1
>> +#define FALLOC_FL_PUNCH_HOLE 2
>> +
>> +#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
>> +#define AT_EMPTY_PATH 0x1000
>> +#endif
>> +
>> +#endif
>> diff --git a/include/sys/param.h b/include/sys/param.h
>> new file mode 100644
>> index 0000000..d877673
>> --- /dev/null
>> +++ b/include/sys/param.h
>> @@ -0,0 +1,8 @@
>> +#ifndef _NEWLIB_GLUE_SYS_PARAM_H_
>> +#define _NEWLIB_GLUE_SYS_PARAM_H_
>> +
>> +#include_next <sys/param.h>
>> +
>> +#define MAXSYMLINKS 20
>> +
>> +#endif
>> 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
      
>
>> +
>> +#define UTIME_NOW  0x3fffffff
>> +#define UTIME_OMIT 0x3ffffffe
>> +
>> +#endif
>> diff --git a/include/sys/uio.h b/include/sys/uio.h
>> index cfec9cc..2b2d22c 100644
>> --- a/include/sys/uio.h
>> +++ b/include/sys/uio.h
>> @@ -54,6 +54,8 @@ extern "C" {
>>   
>>   #define UIO_MAXIOV 1024
>>   
>> +struct iovec { void *iov_base; size_t iov_len; };
>> +
>
> This doesn't follow our general code style.
Copy-paste issues. Fixed

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

-- 
Yuri Volchkov
Software Specialist

NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg

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