Re: [PATCH v3] kexec-tools: Make xc_dlhandle static

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 24, 2018 at 12:04:34PM -0600, Eric DeVolder wrote:
> This patch is a follow-on to commit 894bea93 "kexec-tools: Perform
> run-time linking of libxenctrl.so". This patch addresses feedback
> from Daniel Kiper.
>
> This patch implements Daniel's suggestion to make the
> xc_dlhandle variable static.

You should also mention that by the way you are adding missing extern
in this patch. Or do it in separate patch with relevant comment.

> Signed-off-by: Eric DeVolder <eric.devolder@xxxxxxxxxx>
> Reviewed-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
> ---
> v1: 23jan2018
>  - Implemented feedback from Daniel Kiper
>
> v2: 23jan2018
>  - Implemented feedback from Daniel Kiper
>  - Broke patch into two
>
> v3: 24jan2018
>  - Implemented feedback from Daniel Kiper
>  - Added 'extern' to the new declarations in kexec-xen.h
> ---
>  kexec/kexec-xen.c |  9 ++++++++-
>  kexec/kexec-xen.h | 13 ++++++-------
>  2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
> index 75f8758..960fa16 100644
> --- a/kexec/kexec-xen.c
> +++ b/kexec/kexec-xen.c
> @@ -15,8 +15,15 @@
>  #include "crashdump.h"
>
>  #ifdef CONFIG_LIBXENCTRL_DL
> -void *xc_dlhandle;
> +/* The handle from dlopen(), needed by dlsym(), dlclose() */
> +static void *xc_dlhandle;
>  xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(HYPERCALL_BUFFER_NULL);
> +
> +void *__xc_dlsym(const char *symbol)
> +{
> +	return dlsym(xc_dlhandle, symbol);
> +}
> +
>  xc_interface *__xc_interface_open(xentoollog_logger *logger,
>  				  xentoollog_logger *dombuild_logger,
>  				  unsigned open_flags)
> diff --git a/kexec/kexec-xen.h b/kexec/kexec-xen.h
> index ffb8743..ef08d2c 100644
> --- a/kexec/kexec-xen.h
> +++ b/kexec/kexec-xen.h
> @@ -6,28 +6,27 @@
>
>  #ifdef CONFIG_LIBXENCTRL_DL
>  #include <dlfcn.h>

I understand that you need this header but I think that it is really
needed in kexec/kexec-xen.c only. Is not it? If yes please drop it
from here and add to kexec/kexec-xen.c. Just after "#include <elf.h>".

> -/* The handle from dlopen(), needed by dlsym(), dlclose() */
> -extern void *xc_dlhandle;
> +/* Lookup symbols in libxenctrl.so */
> +extern void *__xc_dlsym(const char *symbol);
>
>  /* Wrappers around xc_interface_open/close() to insert dlopen/dlclose() */
> -xc_interface *__xc_interface_open(xentoollog_logger *logger,
> +extern xc_interface *__xc_interface_open(xentoollog_logger *logger,
>  				  xentoollog_logger *dombuild_logger,
>  				  unsigned open_flags);

Broken variable alignment after extern addition. Please fix this too.

Thanks,

Daniel

_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux