Daniel, thanks for the review, inline responses below.
eric
On 01/23/2018 02:59 PM, Daniel Kiper wrote:
On Tue, Jan 23, 2018 at 01:12:51PM -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.
This patch implements Daniel's suggestion to make the
xc_dlhandle variable static.
Signed-off-by: Eric DeVolder <eric.devolder@xxxxxxxxxx>
Just two nitpicks... Otherwise you can add
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
---
kexec/kexec-xen.c | 9 ++++++++-
kexec/kexec-xen.h | 9 ++++-----
2 files changed, 12 insertions(+), 6 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..8955334 100644
--- a/kexec/kexec-xen.h
+++ b/kexec/kexec-xen.h
@@ -6,9 +6,8 @@
#ifdef CONFIG_LIBXENCTRL_DL
#include <dlfcn.h>
I think that you can drop this include too.
I did check and this is the only place in kexec-tools that includes this
header file (it has the prototypes for dlopen/close/sym()). Without this
header file included, compilation fails. Thus I've left it in.
-
-/* The handle from dlopen(), needed by dlsym(), dlclose() */
-extern void *xc_dlhandle;
+/* Lookup symbols in libxenctrl.so */
+void *__xc_dlsym(const char *symbol);
extern void *__xc_dlsym(const char *symbol);?
Done for this and the others.
But then, IMO, you should put extern for __xc_interface_open()
and __xc_interface_close() below too. You can do it in separate
patch with Reviewed-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
Daniel
Thanks, I've posted v3 of this patch, including your RB.
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
