Re: [Minios-devel] [UNIKRAFT PATCH v2 2/6] lib/nolibc: move shared definitions to special include file

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Reviewed-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>

On 17.08.2018 09:56, Florian Schmidt wrote:
> libc mandates a bunch of definitions that defined from several header
> files. At the same time, these definitions are overlapping, but not
> easily solved by including one libc header file from another. As an
> example (which is fixed in this patch), including stddef.h from stdio.h
> provides stdio.h with the required definitions for size_t, but stddef.h
> contains more definitions that stdio.h should *not* provide. If nolibc
> provides too much, code that compiles with nolibc (because it expects
> these over-generous provision of definitions) will not compile with
> other libc implementations any more.
>
> We take the solution found in other libc implementations such as newlib
> and musl: use __NEED_x macros and include an internal header file with
> definitions that cherry-picks those that we need.
>
> This patch does this for the first three data types. More are to follow.
>
> Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
> ---
>   .../include/nolibc-internal/shareddefs.h      | 57 +++++++++++++++++++
>   lib/nolibc/include/stddef.h                   |  4 +-
>   lib/nolibc/include/stdio.h                    |  4 +-
>   lib/nolibc/include/sys/types.h                |  7 ++-
>   lib/nolibc/include/unistd.h                   |  3 +
>   5 files changed, 70 insertions(+), 5 deletions(-)
>   create mode 100644 lib/nolibc/include/nolibc-internal/shareddefs.h
>
> diff --git a/lib/nolibc/include/nolibc-internal/shareddefs.h 
> b/lib/nolibc/include/nolibc-internal/shareddefs.h
> new file mode 100644
> index 0000000..9fccc15
> --- /dev/null
> +++ b/lib/nolibc/include/nolibc-internal/shareddefs.h
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: BSD-3-Clause */
> +/*
> + * Authors: Florian Schmidt <florian.schmidt@xxxxxxxxx>
> + *
> + * Copyright (c) 2018, NEC Labs Europe, 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.
> + */
> +
> +/* This header does by design not have include guards, so that it can be
> + * included from multiple files. The __NEED_x macros instead make sure that
> + * only those definitions are included that are required by that specific
> + * file, and only if they haven't been defined on a previous pass through
> + * this file.
> + */
> +
> +#include <uk/arch/types.h>
> +
> +#if (defined __NEED_size_t && !defined __DEFINED_size_t)
> +typedef __sz size_t;
> +#define __DEFINED_size_t
> +#endif
> +
> +#if (defined __NEED_ssize_t && !defined __DEFINED_ssize_t)
> +typedef __ssz ssize_t;
> +#define __DEFINED_ssize_t
> +#endif
> +
> +#if (defined __NEED_off_t && !defined __DEFINED_off_t)
> +typedef __off off_t;
> +#define __DEFINED_off_t
> +#endif
> diff --git a/lib/nolibc/include/stddef.h b/lib/nolibc/include/stddef.h
> index a8ed523..1980011 100644
> --- a/lib/nolibc/include/stddef.h
> +++ b/lib/nolibc/include/stddef.h
> @@ -36,7 +36,6 @@
>   #ifndef __STDDEF_H__
>   #define __STDDEF_H__
>   
> -#include <sys/types.h> /* size_t */
>   #include <uk/arch/types.h>
>   
>   #ifdef __cplusplus
> @@ -53,6 +52,9 @@ typedef __sptr ptrdiff_t;
>   #define offsetof(t, d) __offsetof(t, d)
>   #endif
>   
> +#define __NEED_size_t
> +#include <nolibc-internal/shareddefs.h>
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/lib/nolibc/include/stdio.h b/lib/nolibc/include/stdio.h
> index df95c0c..beaf46e 100644
> --- a/lib/nolibc/include/stdio.h
> +++ b/lib/nolibc/include/stdio.h
> @@ -36,13 +36,15 @@
>   #ifndef __STDIO_H__
>   #define __STDIO_H__
>   
> -#include <stddef.h>
>   #include <uk/essentials.h>
>   
>   #ifdef __cplusplus
>   extern "C" {
>   #endif
>   
> +#define __NEED_size_t
> +#include <nolibc-internal/shareddefs.h>
> +
>   struct _nolibc_fd;
>   typedef struct _nolibc_fd FILE;
>   
> diff --git a/lib/nolibc/include/sys/types.h b/lib/nolibc/include/sys/types.h
> index 1d61dc6..9908855 100644
> --- a/lib/nolibc/include/sys/types.h
> +++ b/lib/nolibc/include/sys/types.h
> @@ -43,9 +43,10 @@
>   extern "C" {
>   #endif
>   
> -typedef __sz   size_t;
> -typedef __ssz  ssize_t;
> -typedef __off  off_t;
> +#define __NEED_size_t
> +#define __NEED_ssize_t
> +#define __NEED_off_t
> +#include <nolibc-internal/shareddefs.h>
>   
>   #ifdef __cplusplus
>   }
> diff --git a/lib/nolibc/include/unistd.h b/lib/nolibc/include/unistd.h
> index b723b6a..92d8351 100644
> --- a/lib/nolibc/include/unistd.h
> +++ b/lib/nolibc/include/unistd.h
> @@ -41,6 +41,9 @@
>   extern "C" {
>   #endif
>   
> +#define __NEED_ssize_t
> +#include <nolibc-internal/shareddefs.h>
> +
>   int close(int fd);
>   ssize_t write(int fd, const void *buf, size_t count);
>   ssize_t read(int fd, void *buf, size_t count);
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel