Re: [Minios-devel] [UNIKRAFT/NEWLIB PATCH v2 2/2] lib/posix-crypt: Adapt imported musl code to Unikraft

[Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>]

Reviewed-by: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>

On 27.11.2019 19:08, Roxana Nicolescu wrote:
> Hi Costin,
>
>
> Thanks for the patch. There is one minor thing that I think can be 
> solved during upstream.
> See inline.
>
>
> Best,
>
> Roxana
>
> On 27.11.2019 16:22, Costin Lupu wrote:
> > We tried to keep the changes of the original code to a minimum. For this
> > purpose, we used the C preprocessor in order to disable the 'hidden' 
> > musl
> > qualifier. We also had to rename the <crypt.h> header in
> > `musl-imported/src/include` to <_crypt.h> because, being a private 
> > header, we
> > could not add its directory at the top of the include paths list to 
> > include it
> > before the <crypt.h> header in `musl-imported/include` (the latter being
> > a public include path).
> >
> > Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> > ---
> >   Config.uk                                       |  4 ++++
> >   Makefile.uk                                     | 15 +++++++++++++++
> >   musl-imported/src/crypt/crypt.c                 |  2 +-
> >   musl-imported/src/crypt/crypt_r.c               |  6 +++++-
> >   musl-imported/src/include/{crypt.h => _crypt.h} |  2 +-
> >   5 files changed, 26 insertions(+), 3 deletions(-)
> >   rename musl-imported/src/include/{crypt.h => _crypt.h} (93%)
> >
> > diff --git a/Config.uk b/Config.uk
> > index 5b8f467..ab127f0 100644
> > --- a/Config.uk
> > +++ b/Config.uk
> > @@ -22,4 +22,8 @@ if LIBNEWLIBC
> >       config LIBNEWLIBC_LINUX_ERRNO_EXTENSIONS
> >           bool "Use Linux errno extensions"
> >           default n
> > +
> > +    config LIBNEWLIBC_CRYPT
> > +        bool "Enable crypt() and crypt_r() functions"
> > +        default y
> >   endif
> > diff --git a/Makefile.uk b/Makefile.uk
> > index 9eb1f03..0bcd858 100644
> > --- a/Makefile.uk
> > +++ b/Makefile.uk
> > @@ -68,6 +68,7 @@ LIBNEWLIB_LIBM = 
> > $(LIBNEWLIBC_ORIGIN)/$(LIBNEWLIB_SUBDIR)/newlib/libm
> >   LIBNEWLIBC_COMMON_INCLUDES-y     += -I$(LIBNEWLIBC_BASE)/include
> >   LIBNEWLIBC_COMMON_INCLUDES-y     += 
> > -I$(LIBNEWLIBC_BASE)/musl-imported/include
> >   LIBNEWLIBC_COMMON_INCLUDES-y     += 
> > -I$(LIBNEWLIBC_BASE)/musl-imported/arch/generic
> > +LIBNEWLIBGLUE_CINCLUDES-y        += 
> > -I$(LIBNEWLIBC_BASE)/musl-imported/src/include
> >   LIBNEWLIBC_COMMON_INCLUDES-$(CONFIG_ARCH_X86_64) += 
> > -I$(LIBNEWLIBC_BASE)/musl-imported/arch/x86_64
> >   LIBNEWLIBC_COMMON_INCLUDES-y     += -I$(LIBNEWLIB_LIBC)/include
> >   @@ -132,6 +133,20 @@ LIBNEWLIBGLUE_SRCS-y += 
> > $(LIBNEWLIBC_BASE)/musl-imported/src/misc/syslog.c
> >   LIBNEWLIBGLUE_SRCS-y += 
> > $(LIBNEWLIBC_BASE)/musl-imported/src/termios/tcsetattr.c
> >   LIBNEWLIBGLUE_SRCS-y += 
> > $(LIBNEWLIBC_BASE)/musl-imported/src/termios/tcgetattr.c
> >   +ifeq ($(CONFIG_LIBNEWLIBC_CRYPT),y)
> > +LIBNEWLIBGLUE_CFLAGS-y   += -Wno-missing-braces -Wno-sign-compare
> As we discussed in private, there is an warning `array subscript has 
> type ‘char` which
> can be solved by supressing this type of warnings.
>
> > +LIBNEWLIBGLUE_CFLAGS-y   += -Dhidden=
> > +
> > +LIBNEWLIBGLUE_SRCS-y += 
> > $(LIBNEWLIBC_BASE)/musl-imported/src/crypt/crypt_blowfish.c
> > +LIBNEWLIBGLUE_SRCS-y += 
> > $(LIBNEWLIBC_BASE)/musl-imported/src/crypt/crypt.c
> > +LIBNEWLIBGLUE_SRCS-y += 
> > $(LIBNEWLIBC_BASE)/musl-imported/src/crypt/crypt_des.c
> > +LIBNEWLIBGLUE_SRCS-y += 
> > $(LIBNEWLIBC_BASE)/musl-imported/src/crypt/crypt_md5.c
> > +LIBNEWLIBGLUE_SRCS-y += 
> > $(LIBNEWLIBC_BASE)/musl-imported/src/crypt/crypt_r.c
> > +LIBNEWLIBGLUE_SRCS-y += 
> > $(LIBNEWLIBC_BASE)/musl-imported/src/crypt/crypt_sha256.c
> > +LIBNEWLIBGLUE_SRCS-y += 
> > $(LIBNEWLIBC_BASE)/musl-imported/src/crypt/crypt_sha512.c
> > +LIBNEWLIBGLUE_SRCS-y += 
> > $(LIBNEWLIBC_BASE)/musl-imported/src/crypt/encrypt.c
> > +endif
> > +
> > ################################################################################
> >   # Newlib/libc code -- argz
> > ################################################################################
> > diff --git a/musl-imported/src/crypt/crypt.c 
> > b/musl-imported/src/crypt/crypt.c
> > index e6237e3..4cad99b 100644
> > --- a/musl-imported/src/crypt/crypt.c
> > +++ b/musl-imported/src/crypt/crypt.c
> > @@ -1,5 +1,5 @@
> >   #include <unistd.h>
> > -#include <crypt.h>
> > +#include <_crypt.h>
> >     char *crypt(const char *key, const char *salt)
> >   {
> > diff --git a/musl-imported/src/crypt/crypt_r.c 
> > b/musl-imported/src/crypt/crypt_r.c
> > index db6015e..2e93bd7 100644
> > --- a/musl-imported/src/crypt/crypt_r.c
> > +++ b/musl-imported/src/crypt/crypt_r.c
> > @@ -1,4 +1,4 @@
> > -#include <crypt.h>
> > +#include <_crypt.h>
> >     char *__crypt_r(const char *key, const char *salt, struct 
> > crypt_data *data)
> >   {
> > @@ -20,4 +20,8 @@ char *__crypt_r(const char *key, const char *salt, 
> > struct crypt_data *data)
> >       return __crypt_des(key, salt, output);
> >   }
> >   +/* TODO move this to some global header */
> > +#define weak_alias(old, new) \
> > +    extern __typeof(old) new __attribute__((__weak__, __alias__(#old)))
> > +
> >   weak_alias(__crypt_r, crypt_r);
> > diff --git a/musl-imported/src/include/crypt.h 
> > b/musl-imported/src/include/_crypt.h
> > similarity index 93%
> > rename from musl-imported/src/include/crypt.h
> > rename to musl-imported/src/include/_crypt.h
> > index f6c6309..c3dad71 100644
> > --- a/musl-imported/src/include/crypt.h
> > +++ b/musl-imported/src/include/_crypt.h
> > @@ -3,7 +3,7 @@
> >     #include "../../include/crypt.h"
> >   -#include <features.h>
> > +/* TODO #include <features.h> */
> >     hidden char *__crypt_r(const char *, const char *, struct 
> > crypt_data *);
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel