Re: [PATCH v1] kexec-tools: Tweak run-time handling of libxenctrl.so

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

 



Hi Eric,

On Tue, Jan 23, 2018 at 11:39:29AM -0600, Eric DeVolder wrote:
> This patch is a follow-on to commit 43d3932e "kexec-tools: Perform
> run-time linking of libxenctrl.so". This patch addresses feedback
> from Daniel Kiper concerning the lack of a dlclose() and the desire
> to make xc_dlhandle static, rather than extern.
>
> In the original patch, the call to dlclose() was omitted, in contrast
> to the description in the commit message. This patch inserts the call.
> Note that this dynamic linking feature is dependent upon the proper
> operation of the RTLD_NODELETE flag to dlopen(), which does work as
> advertised on Linux (otherwise the call to dlclose() should be omitted).
>
> This patch also implements Daniel's suggestion to make the
> xc_dlhandle variable static.
>
> This patch has been re-tested per scenarios outlined in the original
> patch commit message.

Please split this patch into two separate patches. One for dlclose() change
and one for static xc_dlhandle change. This will help people who will be
doing git bisect in the future. In general it is much appreciated to
provide one logical change per patch.

> Signed-off-by: Eric DeVolder <eric.devolder@xxxxxxxxxx>
> ---
> v1: 23jan2018
>  - Implemented feedback from Daniel Kiper
> ---
>  kexec/kexec-xen.c | 13 ++++++++++++-
>  kexec/kexec-xen.h |  9 ++++-----
>  2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
> index d42a45a..03ea4a8 100644
> --- a/kexec/kexec-xen.c
> +++ b/kexec/kexec-xen.c
> @@ -15,8 +15,18 @@
>  #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)
> +{
> +	if (xc_dlhandle)
> +		return dlsym(xc_dlhandle, symbol);
> +	else
> +		return NULL;

I am not sure why you need this "if (xc_dlhandle)...". I think that
"return dlsym(xc_dlhandle, symbol);" alone is sufficient here.

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