Xen devel - Mar 2012 - Re: tools/memshr: fix build errors caused by Werror

> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1332942876 -7200
> # Node ID d0fe664fca8a7e7db8b46b2f6c267acc88fa9c78
> # Parent  4bd752a4cdf323c41c50f8cd6286f566d67adeae
> tools/memshr: fix build errors caused by Werror
>
> -O2 -Wall -Werror triggers these warnings:
>
> cc1: warnings being treated as errors
> interface.c: In function ''memshr_daemon_initialize'':
> interface.c:55: error: unused variable ''h''
> interface.c:54: error: unused variable ''shm_base_addr''
> cc1: warnings being treated as errors
> bidir-hash.h:47: error: ''fgprtshr_fgprt_hash'' defined but
not used
> bidir-hash.h:52: error: ''fgprtshr_mfn_hash'' defined but
not used
> bidir-hash.h:57: error: ''fgprtshr_fgprt_cmp'' defined but
not used
> bidir-hash.h:62: error: ''fgprtshr_mfn_cmp'' defined but
not used
> make[3]: *** [interface.o] Error 1
> make[3]: *** [bidir-daemon.o] Error 1
> cc1: warnings being treated as errors
> bidir-hash.h:47: error: ''fgprtshr_fgprt_hash'' defined but
not used
> bidir-hash.h:52: error: ''fgprtshr_mfn_hash'' defined but
not used
> bidir-hash.h:57: error: ''fgprtshr_fgprt_cmp'' defined but
not used
> bidir-hash.h:62: error: ''fgprtshr_mfn_cmp'' defined but
not used
> shm.c:85: error: ''shm_area_close'' defined but not used
>
> Mark fingerprint functions as inline, remove unused shm_area_close.
This looks mostly good except for ....
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>
> diff -r 4bd752a4cdf3 -r d0fe664fca8a tools/memshr/bidir-hash.h
> --- a/tools/memshr/bidir-hash.h
> +++ b/tools/memshr/bidir-hash.h
> @@ -43,22 +43,22 @@ typedef struct vbdblk {
>  #undef BIDIR_VALUE
>  #undef BIDIR_KEY_T
>  #undef BIDIR_VALUE_T
> -static uint32_t fgprtshr_fgprt_hash(uint32_t h)
> +static inline uint32_t fgprtshr_fgprt_hash(uint32_t h)
>  {
>      return h;
>  }
>
> -static uint32_t fgprtshr_mfn_hash(uint64_t m)
> +static inline uint32_t fgprtshr_mfn_hash(uint64_t m)
>  {
>      return (uint32_t)m;
>  }
>
> -static int fgprtshr_fgprt_cmp(uint32_t h1, uint32_t h2)
> +static inline int fgprtshr_fgprt_cmp(uint32_t h1, uint32_t h2)
>  {
>      return (h1 == h2);
>  }
>
> -static int fgprtshr_mfn_cmp(uint32_t m1, uint32_t m2)
> +static inline int fgprtshr_mfn_cmp(uint32_t m1, uint32_t m2)
>  {
>      return (m1 == m2);
>  }
> diff -r 4bd752a4cdf3 -r d0fe664fca8a tools/memshr/interface.c
> --- a/tools/memshr/interface.c
> +++ b/tools/memshr/interface.c
> @@ -51,9 +51,6 @@ void memshr_set_domid(int domid)
>
>  void memshr_daemon_initialize(void)
>  {
> -    void *shm_base_addr;
> -    struct fgprtshr_hash *h;
> -
>      memset(&memshr, 0, sizeof(private_memshr_info_t));
>
>      if((SHARED_INFO = shm_shared_info_open(1)) == NULL)
> diff -r 4bd752a4cdf3 -r d0fe664fca8a tools/memshr/shm.c
> --- a/tools/memshr/shm.c
> +++ b/tools/memshr/shm.c
> @@ -81,12 +81,6 @@ static int shm_area_open(const char *fil
>      return 0;
>  }
>
> -static void shm_area_close(shm_area_t *shma)
> -{
> -    munmap(shma->base_addr, shma->size);
> -    close(shma->fd);
> -}
> -
Removing the close handler is not The Right Thing To Do. Rather, invoke
the close handler where necessary.

Unfortunately, memshr seems blatantly devoid of any cleanup handling. The
problem seems much harder than what it''s worth solving here: exiting
memshr clients need to not leave a pthread_mutex locked in a shared memory
area (ugly, posix named semaphores exist for a reason!), need to agree
that the last one out will destroy the mutex, need to clean up shared
files, etc.

Can we at least atexit shm_area_close?

Andres
>
>  shared_memshr_info_t * shm_shared_info_open(int unlink)
>  {
>
>
>
> ------------------------------
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
>
> End of Xen-devel Digest, Vol 85, Issue 474
> ******************************************
>

On Wed, Mar 28, Andres Lagar-Cavilla wrote:
> > tools/memshr: fix build errors caused by Werror
> > shm.c:85: error: ''shm_area_close'' defined but not
used
> >
> > Mark fingerprint functions as inline, remove unused shm_area_close.
> 
> This looks mostly good except for ....
I will resend this patch without the change to shm_area_close.
Since the tree has alot more issues with -Werror my EXTRA_CFLAGS patch
has to be used with -Wno-error for the time being until issues like this
are resolved with a suitable change.

Olaf