Re: [Minios-devel] [UNIKRAFT/ZLIB, v1, 1/1] Initial port of zlib to Unikraft

[Costin Lupu <costin.lup@xxxxxxxxx>]

Hi Felipe,

I have some minor comments. Please see inline.

On 7/24/19 11:11 AM, Felipe Huici wrote:
> This is a port of zlib to Unikraft as an external library. It requires
> libc, so in your application Makefile the library dependency list
> should read:
> 
>           LIBS := ...:$(UK_LIBS)/newlib:$(UK_LIBS)/zlib:...
> 
> Also make sure that vfscore is selected, along with ramfs and devfs.
> 
> Signed-off-by: Felipe Huici <felipe.huici@xxxxxxxxx>
> ---
>  CODING_STYLE.md |   4 +++
>  CONTRIBUTING.md |   4 +++
>  COPYING.md      |  39 ++++++++++++++++++++++
>  Config.uk       |   4 +++
>  MAINTAINERS.md  |  10 ++++++
>  Makefile.uk     |  79 +++++++++++++++++++++++++++++++++++++++++++
>  README.md       |  11 ++++++
>  exportsyms.uk   | 102 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  8 files changed, 253 insertions(+)
>  create mode 100644 CODING_STYLE.md
>  create mode 100644 CONTRIBUTING.md
>  create mode 100644 COPYING.md
>  create mode 100644 Config.uk
>  create mode 100644 MAINTAINERS.md
>  create mode 100644 Makefile.uk
>  create mode 100644 README.md
>  create mode 100644 exportsyms.uk
> 
> diff --git a/CODING_STYLE.md b/CODING_STYLE.md
> new file mode 100644
> index 0000000..5730041
> --- /dev/null
> +++ b/CODING_STYLE.md
> @@ -0,0 +1,4 @@
> +Coding Style
> +============
> +
> +Please refer to the `CODING_STYLE.md` file in the main Unikraft repository.
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> new file mode 100644
> index 0000000..5f55eca
> --- /dev/null
> +++ b/CONTRIBUTING.md
> @@ -0,0 +1,4 @@
> +Contributing to Unikraft
> +=======================
> +
> +Please refer to the `CONTRIBUTING.md` file in the main Unikraft repository.
> diff --git a/COPYING.md b/COPYING.md
> new file mode 100644
> index 0000000..973051a
> --- /dev/null
> +++ b/COPYING.md
> @@ -0,0 +1,39 @@
> +License
> +=======
> +
> +Unikraft zlib wrappers
> +------------------------
> +
> +This repository contains wrapper code to build zlib with Unikraft.
> +Each C code file in this repository should declare who is the
> +copyright owner and under which terms and conditions the code is
> +licensed. If such a licence note is missing, the following copyright
> +notice will apply:
> +
> +     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.
> +
> diff --git a/Config.uk b/Config.uk
> new file mode 100644
> index 0000000..6e5a75f
> --- /dev/null
> +++ b/Config.uk
> @@ -0,0 +1,4 @@
> +menuconfig LIBZLIB
> +        bool "zlib - a compression library"
> +        default y
> +        depends on HAVE_LIBC
> diff --git a/MAINTAINERS.md b/MAINTAINERS.md
> new file mode 100644
> index 0000000..5a4abc4
> --- /dev/null
> +++ b/MAINTAINERS.md
> @@ -0,0 +1,10 @@
> +Maintainers List
> +================
> +
> +For notes on how to read this information, please refer to `MAINTAINERS.md` 
> in
> +the main Unikraft repository.
> +
> +     LIBUUID-UNIKRAFT
> +     M:      Felipe Huici <felipe.huici@xxxxxxxxx>
> +     L:      minios-devel@xxxxxxxxxxxxx
> +     F: *
> diff --git a/Makefile.uk b/Makefile.uk
> new file mode 100644
> index 0000000..de7e182
> --- /dev/null
> +++ b/Makefile.uk
> @@ -0,0 +1,79 @@
> +#  libzlib Makefile.uc
> +#
> +#  Authors: Felipe Huici <felipe.huici@xxxxxxxxx>
> +#
> +#
> +#  Copyright (c) 2017, NEC Europe Ltd., NEC Corporation. All rights reserved.

s/2017/2019

> +#
> +#  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,libzlib,$(CONFIG_LIBZLIB)))
> +
> +################################################################################
> +# Sources
> +################################################################################
> +LIBZLIB_VERSION=1.2.11
> +LIBZLIB_URL=http://www.zlib.net/zlib-$(LIBZLIB_VERSION).tar.gz
> +LIBZLIB_DIR=zlib-$(LIBZLIB_VERSION)
> +
> +LIBZLIB_PATCHDIR=$(LIBZLIB_BASE)/patches
> +$(eval $(call fetch,libzlib,$(LIBZLIB_URL),$(LIBZLIB_VERSION).tar.gz))
> +$(eval $(call patch,libzlib,$(LIBZLIB_PATCHDIR),$(LIBZLIB_DIR)))
> +
> +################################################################################
> +# Helpers
> +################################################################################
> +LIBZLIB=$(LIBZLIB_ORIGIN)/$(LIBZLIB_DIR)
> +
> +################################################################################
> +# Library includes
> +################################################################################
> +CINCLUDES-$(CONFIG_LIBZLIB) += -I$(LIBZLIB)     
> +
> +################################################################################
> +# Sources
> +################################################################################
> +LIBZLIB_SRCS-y += $(LIBZLIB)/inflate.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/gzclose.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/zutil.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/crc32.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/uncompr.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/gzread.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/trees.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/gzlib.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/infback.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/gzwrite.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/compress.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/inftrees.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/deflate.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/adler32.c
> +LIBZLIB_SRCS-y += $(LIBZLIB)/inffast.c
> diff --git a/README.md b/README.md
> new file mode 100644
> index 0000000..b5a1e78
> --- /dev/null
> +++ b/README.md
> @@ -0,0 +1,11 @@
> +libzlib for Unikraft
> +===================
> +This is a port of zlib to Unikraft. It requires newlib, so in your
> +application Makefile the library dependency list should read:
> +
> +     LIBS := ...:$(UK_LIBS)/newlib:$(UK_LIBS)/zlib:...
> +
> +Also make sure that vfscore is selected, along with ramfs and devfs.

You should select the dependencies in the Config.uk file and remove this
line from README.

> +
> +Please refer to the `README.md` as well as the documentation in the `doc/`
> +subdirectory of the main unikraft repository for further information.
> diff --git a/exportsyms.uk b/exportsyms.uk
> new file mode 100644
> index 0000000..e674788
> --- /dev/null
> +++ b/exportsyms.uk
> @@ -0,0 +1,102 @@
> +deflateInit
> +deflate
> +deflateEnd
> +inflateInit
> +inflate
> +inflateEnd
> +deflateInit2
> +deflateSetDictionary
> +deflateGetDictionary
> +deflateCopy
> +deflateReset
> +deflateParams
> +deflateTune
> +deflateBound
> +deflatePending
> +deflatePrime
> +deflateSetHeader
> +inflateInit2
> +inflateSetDictionary
> +inflateGetDictionary
> +inflateSync
> +inflateCopy
> +inflateReset
> +inflateReset2
> +inflatePrime
> +inflateMark
> +inflateGetHeader
> +inflateBackInit
> +inflateBack
> +inflateBackEnd
> +zlibCompileFlags
> +compress
> +compress2
> +compressBound
> +uncompress
> +uncompress2
> +gzopen
> +gzdopen
> +gzbuffer
> +gzsetparams
> +gzread
> +gzfread
> +gzwrite
> +gzfwrite
> +gzprintf
> +gzputs
> +gzputc
> +gzgetc
> +gzungetc
> +gzflush
> +gzseek
> +gzrewind
> +gztell
> +gzoffset
> +gzeof
> +gzdirect
> +gzclose
> +gzclose_r
> +gzclose_w
> +gzclearerr
> +adler32
> +adler32_z
> +adler32_combine
> +crc32
> +crc32_z
> +crc32_combine
> +deflateInit_
> +inflateInit_
> +deflateInit2_
> +inflateInit2_
> +inflateBackInit_
> +gzgetc_
> +gzopen64
> +gzseek64
> +gztell64
> +gzoffset64
> +adler32_combine64
> +crc32_combine64
> +gzopen64
> +gzseek64
> +gztell64
> +gzoffset64
> +adler32_combine64
> +crc32_combine64
> +gzopen
> +gzseek
> +gztell
> +gzoffset
> +adler32_combine
> +crc32_combine
> +adler32_combine
> +crc32_combine
> +inflateSyncPoint
> +inflateUndermine
> +inflateValidate
> +inflateResetKeep
> +deflateResetKeep
> +gzopen_w
> +gzvprintf
> +gzerror
> +zlibVersion
> +gzgets
> \ No newline at end of file
> 

From what I see in the manual [1], the symbols ending with underscore
should not be exported. Also, you have some duplicates here, perhaps it
should help putting the symbols sorted alphabetically here. Also, you're
missing the following symbols:

* get_crc_table
* inflateCodesUsed
* zError


Cheers,
Costin

[1] https://www.zlib.net/manual.html

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