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

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

 



Inline responses below. v4 forthcoming.
eric

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

I've included this in the v4 message. I've removed your RB too.


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>".

Ah yes, you are correct. Now that dlopen/close/sym() are all wrapped in functions in kexec-xen.c, this header can be included just in that file.

done


-/* 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.

done.


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