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

Re: [Minios-devel] [UNIKRAFT/LIBUUID PATCH 2/2] Initial port of the libuuid library (version 1.0.3).


  • To: Sharan Santhanam <Sharan.Santhanam@xxxxxxxxx>, "minios-devel@xxxxxxxxxxxxx" <minios-devel@xxxxxxxxxxxxx>
  • From: Felipe Huici <Felipe.Huici@xxxxxxxxx>
  • Date: Fri, 12 Apr 2019 12:36:11 +0000
  • Accept-language: en-US, de-DE
  • Delivery-date: Fri, 12 Apr 2019 12:38:32 +0000
  • List-id: Mini-os development list <minios-devel.lists.xenproject.org>
  • Thread-index: AQHU7rhZAoQqSM1PvU+yMK6ZmQkeFKY1QumAgAM4RIA=
  • Thread-topic: [Minios-devel] [UNIKRAFT/LIBUUID PATCH 2/2] Initial port of the libuuid library (version 1.0.3).

Hi Sharan,

Thanks for the review! My answers are inlined.

-- Felipe

On 10.04.19, 15:26, "Sharan Santhanam" <Sharan.Santhanam@xxxxxxxxx> wrote:

    Hello Felipe,
    
    Please find the review comments inline:
    
    * exportsyms.uk is missing for this library
    
> Ok, I'll add it.
    
    Thanks & Regards
    Sharan
    
    On 4/9/19 11:40 AM, Felipe Huici wrote:
    > Note newlib is required.
    > 
    > Signed-off-by: Felipe Huici <felipe.huici@xxxxxxxxx>
    > ---
    >   Config.uk                                      |   6 +
    >   Makefile.uk                                    |  92 +++++++++++++++
    >   include/config.h                               | 153 
+++++++++++++++++++++++++
    >   patches/0001-add-syscall-h-compile-guard.patch |  12 ++
    >   4 files changed, 263 insertions(+)
    >   create mode 100644 Config.uk
    >   create mode 100644 Makefile.uk
    >   create mode 100644 include/config.h
    >   create mode 100644 patches/0001-add-syscall-h-compile-guard.patch
    > 
    > diff --git a/Config.uk b/Config.uk
    > new file mode 100644
    > index 0000000..4de0d31
    > --- /dev/null
    > +++ b/Config.uk
    > @@ -0,0 +1,6 @@
    > +menuconfig LIBUUID
    > +    bool "uuid - library for unique id generation"
    > +    default n
    > +    select HAVE_LIBC
    
    select  UKUNISTD  missing?
    
> Yes, I'll add it.

    > +
    > +
    > diff --git a/Makefile.uk b/Makefile.uk
    > new file mode 100644
    > index 0000000..a0511f8
    > --- /dev/null
    > +++ b/Makefile.uk
    > @@ -0,0 +1,92 @@
    > +#  libuuid Makefile.uc
    > +#
    > +#  Authors: Felipe Huici <felipe.huici@xxxxxxxxx>
    > +#
    > +#
    > +#  Copyright (c) 2019, NEC Europe Ltd., NEC Corporation. All rights 
reserved.
    > +#
    > +#  Redistribution and use in source and binary forms, with or without
    > +#  modification, are permitted provided that the following conditions
    > +#  are met:
    > +#
    > +#  1. Redistributions of source code must retain the above copyright
    > +#     notice, this list of conditions and the following disclaimer.
    > +#  2. Redistributions in binary form must reproduce the above copyright
    > +#     notice, this list of conditions and the following disclaimer in the
    > +#     documentation and/or other materials provided with the 
distribution.
    > +#  3. Neither the name of the copyright holder nor the names of its
    > +#     contributors may be used to endorse or promote products derived 
from
    > +#     this software without specific prior written permission.
    > +#
    > +#  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS 
"AS IS"
    > +#  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, 
THE
    > +#  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR 
PURPOSE
    > +#  ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR 
CONTRIBUTORS BE
    > +#  LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
    > +#  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
    > +#  SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR 
BUSINESS
    > +#  INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER 
IN
    > +#  CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR 
OTHERWISE)
    > +#  ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED 
OF THE
    > +#  POSSIBILITY OF SUCH DAMAGE.
    > +#
    > +#  THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
    > +#
    > +
    > 
+################################################################################
    > +# Library registration
    > 
+################################################################################
    > +$(eval $(call addlib_s,libuuid,$(CONFIG_LIBUUID)))
    > +
    > 
+################################################################################
    > +# Sources
    > 
+################################################################################
    > +LIBUUID_VERSION=1.0.3
    
    
LIBUUID_URL=https://sourceforge.net/projects/libuuid/files/libuuid-$(LIBUUID_VERSION).tar.gz/download
 
    
> Will fix, thanks.

    > 
+LIBUUID_URL=https://sourceforge.net/projects/libuuid/files/libuuid-1.0.3.tar.gz/download
    > +LIBUUID_PATCHDIR=$(LIBUUID_BASE)/patches
    > +$(eval $(call fetchas,libuuid,$(LIBUUID_URL),$(LIBUUID_VERSION).tgz))
    > +$(eval $(call 
patch,libuuid,$(LIBUUID_PATCHDIR),libuuid-$(LIBUUID_VERSION)))
    > +
    > 
+################################################################################
    > +# Helpers
    > 
+################################################################################
    > +LIBUUID_SUBDIR=libuuid-$(LIBUUID_VERSION)
    > +LIBUUID_SRC=$(LIBUUID_ORIGIN)/$(LIBUUID_SUBDIR)
    > +
    > 
+################################################################################
    > +# Library includes
    > 
+################################################################################
    
    
    Not sure if we should include LIBUUID_SRC here. 
     
    
     From the source Makefile.in +360, only 
     
    
    uuid.h is exported and it configured uuid/uuid.h. So we 
     
    
    ## must stick with that path.

> Ok, I'll fix it.

    > +CINCLUDES-$(CONFIG_LIBUUID) += -I$(LIBUUID_BASE)/include \
    > +                               -I$(LIBUUID_SRC)
    > +
    > 
+################################################################################
    > +# Global flags
    > 
+################################################################################
    > +LIBUUID_CFLAGS-y += -DHAVE_CONFIG_H
    > +
    > +# Suppress some warnings to make the build process look neater
    > +SUPPRESS_FLAGS += -Wno-unused-parameter -Wno-unused-variable 
-Wno-nonnull       \
    > +-Wno-unused-but-set-variable -Wno-unused-label -Wno-char-subscripts      
       \
    > +-Wno-unused-function -Wno-missing-field-initializers -Wno-uninitialized  
       \
    > +-Wno-array-bounds -Wno-maybe-uninitialized -Wno-pointer-sign 
-Wno-unused-value  \
    > +-Wno-unused-macros -Wno-parentheses -Wno-implicit-function-declaration   
       \
    > +-Wno-missing-braces -Wno-endif-labels -Wno-unused-but-set-variable       
       \
    > +-Wno-implicit-function-declaration -Wno-type-limits -Wno-sign-compare
    > +
    > +LIBUUID_CFLAGS-y   += $(SUPPRESS_FLAGS)
    > +LIBUUID_CXXFLAGS-y += $(SUPPRESS_FLAGS)
    > +
    > 
+################################################################################
    > +# Sources
    > 
+################################################################################
    > +LIBUUID_SRCS-y += $(LIBUUID_SRC)/clear.c
    > +LIBUUID_SRCS-y += $(LIBUUID_SRC)/copy.c
    > +LIBUUID_SRCS-y += $(LIBUUID_SRC)/isnull.c
    > +LIBUUID_SRCS-y += $(LIBUUID_SRC)/parse.c
    > +LIBUUID_SRCS-y += $(LIBUUID_SRC)/unparse.c
    > +LIBUUID_SRCS-y += $(LIBUUID_SRC)/compare.c
    > +LIBUUID_SRCS-y += $(LIBUUID_SRC)/gen_uuid.c
    > +LIBUUID_SRCS-y += $(LIBUUID_SRC)/pack.c
    > +LIBUUID_SRCS-y += $(LIBUUID_SRC)/randutils.c
    > +LIBUUID_SRCS-y += $(LIBUUID_SRC)/unpack.c
    > +LIBUUID_SRCS-y += $(LIBUUID_SRC)/uuid_time.c
    > diff --git a/include/config.h b/include/config.h
    > new file mode 100644
    > index 0000000..ddad5df
    > --- /dev/null
    > +++ b/include/config.h
    > @@ -0,0 +1,153 @@
    > +/* config.h.  Generated from config.h.in by libuuid's configure.  */
    > +/* config.h.in.  Generated from configure.ac by autoheader.  */
    > +
    
    This is reading the sysconf. We don't support sysconf.

> Ok.

    > +/* Define to 1 if you have the declaration of `_SC_HOST_NAME_MAX', and 
to 0 if
    > +   you don't. */
    > +#define HAVE_DECL__SC_HOST_NAME_MAX 1
    > +
    
    This add dynamic loading support. Which not required for us

> Ok.

    > +/* Define to 1 if you have the <dlfcn.h> header file. */
    > +#define HAVE_DLFCN_H 1
    > +
    > +/* Define to 1 if you have the <fcntl.h> header file. */
    > +#define HAVE_FCNTL_H 1
    > +
    > +/* Define to 1 if you have the `ftruncate' function. */
    > +#define HAVE_FTRUNCATE 1
    > +
    > +/* Define to 1 if you have the `gettimeofday' function. */
    > +#define HAVE_GETTIMEOFDAY 1
    > +
    > +/* Define to 1 if you have the <inttypes.h> header file. */
    > +#define HAVE_INTTYPES_H 1
    > +
    > +/* Define to 1 if you have the <limits.h> header file. */
    > +#define HAVE_LIMITS_H 1
    > +
    > +/* Define to 1 if you have the <memory.h> header file. */
    > +#define HAVE_MEMORY_H 1
    > +
    > +/* Define to 1 if you have the `memset' function. */
    > +#define HAVE_MEMSET 1
    > +
    
    netinet/in.h - Should be added only LWIP is present?

> Yes.

    > +/* Define to 1 if you have the <netinet/in.h> header file. */
    > +#define HAVE_NETINET_IN_H 1
    > +
    
    HAVE_SOCKET : Should be linked with LWIP
    > +/* Define to 1 if you have the `socket' function. */
    > +#define HAVE_SOCKET 1
    > +
    
    HAVE_SRANDOM: Should be linked with srandom implementation in newlib or 
    other libc

> Ok.

    > +/* Define to 1 if you have the `srandom' function. */
    > +#define HAVE_SRANDOM 1
    > +
    > +/* Define to 1 if you have the <stdint.h> header file. */
    > +#define HAVE_STDINT_H 1
    > +
    > +/* Define to 1 if you have the <stdlib.h> header file. */
    > +#define HAVE_STDLIB_H 1
    > +
    > +/* Define to 1 if you have the <strings.h> header file. */
    > +#define HAVE_STRINGS_H 1
    > +
    > +/* Define to 1 if you have the <string.h> header file. */
    > +#define HAVE_STRING_H 1
    > +
    > +/* Define to 1 if you have the `strtoul' function. */
    > +#define HAVE_STRTOUL 1
    > +
    > +/* Define to 1 if you have the <sys/file.h> header file. */
    > +#define HAVE_SYS_FILE_H 1
    > +
    > +/* Define to 1 if you have the <sys/ioctl.h> header file. */
    > +#define HAVE_SYS_IOCTL_H 1
    > +
    
      Should this be defined based on lwip

> Yes.

    > +/* Define to 1 if you have the <sys/socket.h> header file. */
    > +#define HAVE_SYS_SOCKET_H 1
    > +
    > +/* Define to 1 if you have the <sys/stat.h> header file. */
    > +#define HAVE_SYS_STAT_H 1
    > +
    > +/* Define to 1 if you have the <sys/time.h> header file. */
    > +#define HAVE_SYS_TIME_H 1
    > +
    > +/* Define to 1 if you have the <sys/types.h> header file. */
    > +#define HAVE_SYS_TYPES_H 1
    > +
    > +/* Define to 1 if you have the <unistd.h> header file. */
    > +#define HAVE_UNISTD_H 1
    > +
    > +/* Define to 1 if you have the `usleep' function. */
    > +#define HAVE_USLEEP 1
    > +
    
    Do we need these configuration(s)?

> I suspect so, but I'll check.

Thanks,

-- Felipe

    > +/* Define to the sub-directory in which libtool stores uninstalled 
libraries.
    > +   */
    > +#define LT_OBJDIR ".libs/"
    > +
    > +/* Name of package */
    > +#define PACKAGE "libuuid"
    > +
    > +/* Define to the address where bug reports for this package should be 
sent. */
    > +#define PACKAGE_BUGREPORT "sloowfranklin@xxxxxxxxx"
    > +
    > +/* Define to the full name of this package. */
    > +#define PACKAGE_NAME "libuuid"
    > +
    > +/* Define to the full name and version of this package. */
    > +#define PACKAGE_STRING "libuuid 1.0.3"
    > +
    > +/* Define to the one symbol short name of this package. */
    > +#define PACKAGE_TARNAME "libuuid"
    > +
    > +/* Define to the home page for this package. */
    > +#define PACKAGE_URL ""
    > +
    > +/* Define to the version of this package. */
    > +#define PACKAGE_VERSION "1.0.3"
    > +
    > +/* Define to 1 if you have the ANSI C header files. */
    > +#define STDC_HEADERS 1
    > +
    > +/* Version number of package */
    > +#define VERSION "1.0.3"
    > +
    > +/* Define for Solaris 2.5.1 so the uint32_t typedef from <sys/synch.h>,
    > +   <pthread.h>, or <semaphore.h> is not used. If the typedef were 
allowed, the
    > +   #define below would cause a syntax error. */
    > +/* #undef _UINT32_T */
    > +
    > +/* Define for Solaris 2.5.1 so the uint64_t typedef from <sys/synch.h>,
    > +   <pthread.h>, or <semaphore.h> is not used. If the typedef were 
allowed, the
    > +   #define below would cause a syntax error. */
    > +/* #undef _UINT64_T */
    > +
    > +/* Define for Solaris 2.5.1 so the uint8_t typedef from <sys/synch.h>,
    > +   <pthread.h>, or <semaphore.h> is not used. If the typedef were 
allowed, the
    > +   #define below would cause a syntax error. */
    > +/* #undef _UINT8_T */
    > +
    > +/* Define to the type of a signed integer type of width exactly 32 bits 
if
    > +   such a type exists and the standard includes do not define it. */
    > +/* #undef int32_t */
    > +
    > +/* Define to `int' if <sys/types.h> does not define. */
    > +/* #undef mode_t */
    > +
    > +/* Define to `unsigned int' if <sys/types.h> does not define. */
    > +/* #undef size_t */
    > +
    > +/* Define to `int' if <sys/types.h> does not define. */
    > +/* #undef ssize_t */
    > +
    > +/* Define to the type of an unsigned integer type of width exactly 16 
bits if
    > +   such a type exists and the standard includes do not define it. */
    > +/* #undef uint16_t */
    > +
    > +/* Define to the type of an unsigned integer type of width exactly 32 
bits if
    > +   such a type exists and the standard includes do not define it. */
    > +/* #undef uint32_t */
    > +
    > +/* Define to the type of an unsigned integer type of width exactly 64 
bits if
    > +   such a type exists and the standard includes do not define it. */
    > +/* #undef uint64_t */
    > +
    > +/* Define to the type of an unsigned integer type of width exactly 8 
bits if
    > +   such a type exists and the standard includes do not define it. */
    > +/* #undef uint8_t */
    > diff --git a/patches/0001-add-syscall-h-compile-guard.patch 
b/patches/0001-add-syscall-h-compile-guard.patch
    > new file mode 100644
    > index 0000000..adea66a
    > --- /dev/null
    > +++ b/patches/0001-add-syscall-h-compile-guard.patch
    > @@ -0,0 +1,12 @@
    > +--- a/randutils.c        2019-04-03 14:46:14.827682485 +0200
    > ++++ b/randutils.c        2019-04-03 14:46:48.375286950 +0200
    > +@@ -13,7 +13,9 @@
    > + #include <string.h>
    > + #include <sys/time.h>
    > +
    > ++#ifdef DO_JRAND_MIX
    > + #include <sys/syscall.h>
    > ++#endif
    > +
    > + #include "randutils.h"
    > +
    > 
    

_______________________________________________
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®.