|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT/LIBUUID PATCH v2 2/2] Initial port of the libuuid library (version 1.0.3).
Hey Felipe,I have some comments for this patch. Maybe I need they need a bit more clarification. Thanks, Simon On 15.04.19 10:21, Felipe Huici wrote: Note newlib is required. Signed-off-by: Felipe Huici <felipe.huici@xxxxxxxxx> --- Config.uk | 5 ++ Makefile.uk | 100 +++++++++++++++++++++++++ exportsyms.uk | 19 +++++ include/config.h | 77 +++++++++++++++++++ patches/0001-add-syscall-h-compile-guard.patch | 12 +++ 5 files changed, 213 insertions(+) create mode 100644 Config.uk create mode 100644 Makefile.uk create mode 100644 exportsyms.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..c6d0769 --- /dev/null +++ b/Config.uk @@ -0,0 +1,5 @@ +menuconfig LIBUUID + bool "libuuid - library for unique id generation" + default n + select HAVE_LIBC You should use "depends on HAVE_LIBC" instead. We introduced these HAVE_* feature variables to communicate that some "standard" features are provided by a library. Your line is stating that LIBUUID is a libc "provider" (see libs/Config.uk). I know, this stuff is confusing. + select UKUNISTD Shouldn't this be LIBUKUNISTD? diff --git a/Makefile.uk b/Makefile.uk new file mode 100644 index 0000000..ed1bc39 --- /dev/null +++ b/Makefile.uk @@ -0,0 +1,100 @@ +# 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 +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 +################################################################################ +CINCLUDES-$(CONFIG_LIBUUID) += -I$(LIBUUID_BASE)/include Oh, this is making <config.h> available also outside of this library. Are you sure you want this? I think this is dangerous and will potentially clash with other libraries later. I actually would expect that this config header is only used within the library scope. + +################################################################################ +# 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 Are you sure you want all of them? I am especially concerned about -Wno-array-bounds, -Wno-type-limits, and -Wno-sign-compare. You should also namespace your SUPPRESS_FLAGS variable name: + +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 + +################################################################################ +# Lib-specific Targets - ensure users can #include <uuid/uuid.h> +################################################################################ +$(LIBUUID_BUILD)/.prepared: + $(call verbose_cmd,CONFIGURE,libuuid: $@,\ Instead of CONFIGURE, you should use LN as command description. You probably can also use build_cmd instead of verbose_cmd. + ln -s $(LIBUUID_SRC) $(LIBUUID_BASE)/include/uuid && \ + $(TOUCH) $@) Instead of this I would actually create a new library sub build directory and link only necessary public headers there. This way you do a much cleaner separation of internal and external headers:
$(call mk_sub_build_dir,libuuid,include/uuid)
$(LIBUUID_BUILD)/.prepared:
$(call verbose_cmd,LN,libuuid: header1.h,\
ln -s $(LIBUUID_SRC)/header1.h \
$(call sub_build_dir,libuuid,include)/uuid/header1.h)
$(call verbose_cmd,LN,libuuid: header2.h,\
ln -s $(LIBUUID_SRC)/header2.h \
$(call sub_build_dir,libuuid,include)/uuid/header2.h)
@$(TOUCH) $@
You then register this new include folder globally:
CINCLUDES-$(CONFIG_LIBUUID) += -I$(call sub_build_dir,libuuid,include)
Please check this code example, it may not work because I did not tested
it. I just wanted to give an idea.
+UK_PREPARE += $(LIBUUID_BUILD)/.prepared diff --git a/exportsyms.uk b/exportsyms.uk new file mode 100644 index 0000000..e94d21e --- /dev/null +++ b/exportsyms.uk @@ -0,0 +1,19 @@ +uuid_clear +uuid_compare +uuid_copy +uuid_generate +__uuid_generate_random Is __uuid_generate_random really a public interfaces provided by the library? I guess it is not because there is also uuid_generate_random. +uuid_generate_random +__uuid_generate_time Is __uuid_generate_time really a public interfaces provided by the library? +uuid_generate_time +uuid_generate_time_safe +uuid_is_null +uuid_pack +uuid_parse +uuid_time +uuid_type +uuid_unpack +uuid_unparse +uuid_unparse_lower +uuid_unparse_upper +uuid_variant diff --git a/include/config.h b/include/config.h new file mode 100644 index 0000000..b818210 --- /dev/null +++ b/include/config.h @@ -0,0 +1,77 @@ +/* config.h. Generated from config.h.in by libuuid's configure. */ +/* config.h.in. Generated from configure.ac by autoheader. */ + +/* 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 + +/* Define to 1 if you have the <netinet/in.h> header file. */ +#ifndef $(CONFIG_LIBLWIP) Shouldn't this be the opposite way around?: #ifdef $(CONFIG_LIBLWIP)Btw, you could also depend on the generic feature flag: $(HAVE_NW_STACK) instead. +#define HAVE_NETINET_IN_H 1 +#endif + +/* Define to 1 if you have the `socket' function. */ +#ifndef $(CONFIG_LIBLWIP) Same here. +#define HAVE_SOCKET 1 +#endif + +/* 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 + +/* Define to 1 if you have the <sys/socket.h> header file. */ +#ifndef $(CONFIG_LIBLWIP) +#define HAVE_SYS_SOCKET_H 1 +#endif ...and here again. ;-) + +/* 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 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
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |