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

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

 



Thanks Daniel, inline responses below.
Eric

On 01/23/2018 12:15 PM, Daniel Kiper wrote:
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.

Done. I've posted a v2 with these as separate patches.


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.

I tested and dlsym() can gracefully accept NULL as the handle, and return NULL. The dlclose() segfaults with a NULL handle. I've made
the change.


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