Re: [Minios-devel] [UNIKRAFT PATCH v2 1/2] include/plat: Remove plat/common/include/memory.h

["Justin He (Arm Technology China)" <Justin.He@xxxxxxx>]

Hi Costin

> -----Original Message-----
> From: Minios-devel <minios-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf
> Of Costin Lupu
> Sent: Saturday, October 12, 2019 7:12 PM
> To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; minios-
> devel@xxxxxxxxxxxxxxxxxxxx; Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Cc: Felipe Huici <felipe.huici@xxxxxxxxx>; Kaly Xin (Arm Technology China)
> <Kaly.Xin@xxxxxxx>; Julien Grall <Julien.Grall@xxxxxxx>;
> Sharan.Santhanam@xxxxxxxxx; Santiago.Pagani@xxxxxxxxx; nd
> <nd@xxxxxxx>
> Subject: Re: [Minios-devel] [UNIKRAFT PATCH v2 1/2] include/plat: Remove
> plat/common/include/memory.h
> 
> Hi Jia He,
> 
> I agree that this warning has to be fixed. However, we cannot fix it by
> removing `plat/common/include/memory.h`, we should find another
> solution
> and this is why.
> 
> Headers under `include` directory, such as `include/uk/plat/memory.h`,
> declare functions that are part of the platform public API. Headers
> under `plat/common/include/` declare functions that are used by more
> platforms, but that are not public. Therefore, we cannot make
> `_ukplat_mem_mappings_init()` public because that wasn't the intention
> behind defining it.
> 
> Maybe we should think again whether we really need to include newlib's
> memory.h header in platform code and, if not, then find an elegant way
> to skip including that one.
If we can't remove include/uk/plat/memory.h, how about rename it with another
name? Otherwise this might be conflicted with newlibc/.../memory.h.


--
Cheers,
Justin (Jia He)


> 
> Cheers,
> Costin
> 
> On 10/12/19 9:39 AM, Jia He wrote:
> > When I enable newlibc, there is a compilation warning:
> > plat/common/memory.c: In function 'ukplat_memallocator_set':
> > plat/common/memory.c:49:2: warning: implicit declaration of function
> > '_ukplat_mem_mappings_init'; did you mean 'ukplat_memregion_get'?
> > [-Wimplicit-function-declaration]
> >   _ukplat_mem_mappings_init();
> >   ^~~~~~~~~~~~~~~~~~~~~~~~~
> >   ukplat_memregion_get
> >
> > The root cause is there is a memory.h under newlibc including directory:
> > ./build/libnewlibc/origin/newlib-
> 2.5.0.20170922/newlib/libc/include/memory.h
> >
> > This fixes it by moving declaration of _ukplat_mem_mappings_init from
> > plat/common/include/memory.h to . Then remove
> > the former file.
> >
> > Signed-off-by: Jia He <justin.he@xxxxxxx>
> > ---
> >  include/uk/plat/memory.h     |  9 +++++++
> >  plat/common/include/memory.h | 47 ------------------------------------
> >  2 files changed, 9 insertions(+), 47 deletions(-)
> >  delete mode 100644 plat/common/include/memory.h
> >
> > diff --git a/include/uk/plat/memory.h b/include/uk/plat/memory.h
> > index 4c4e4dc..df9ce19 100644
> > --- a/include/uk/plat/memory.h
> > +++ b/include/uk/plat/memory.h
> > @@ -164,6 +164,15 @@ struct uk_alloc *ukplat_memallocator_get(void);
> >   */
> >  void ukplat_stack_set_current_thread(void *thread_addr);
> >
> > +/**
> > + * Initializes the platform memory mappings which require an allocator.
> This
> > + * function must always be called after initializing a memory allocator
> and
> > + * before initializing the subsystems that require memory allocation. It is
> an
> > + * internal function common to all platforms.
> > + * @return 0 on success, < 0 otherwise
> > + */
> > +int _ukplat_mem_mappings_init(void);
> > +
> >  #ifdef __cplusplus
> >  }
> >  #endif
> > diff --git a/plat/common/include/memory.h
> b/plat/common/include/memory.h
> > deleted file mode 100644
> > index f627348..0000000
> > --- a/plat/common/include/memory.h
> > +++ /dev/null
> > @@ -1,47 +0,0 @@
> > -/* SPDX-License-Identifier: BSD-3-Clause */
> > -/*
> > - * Authors: Costin Lupu <costin.lupu@xxxxxxxxx>
> > - *
> > - * Copyright (c) 2018, 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.
> > - */
> > -
> > -#ifndef __PLAT_CMN_MEMORY_H__
> > -#define __PLAT_CMN_MEMORY_H__
> > -
> > -/**
> > - * Initializes the platform memory mappings which require an allocator.
> This
> > - * function must always be called after initializing a memory allocator
> and
> > - * before initializing the subsystems that require memory allocation. It is
> an
> > - * internal function common to all platforms.
> > - * @return 0 on success, < 0 otherwise
> > - */
> > -int _ukplat_mem_mappings_init(void);
> > -
> > -#endif /* __PLAT_CMN_MEMORY_H__ */
> >
> 
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel