[PATCH v3] kexec-tools: Make xc_dlhandle static

[Eric DeVolder]

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 at oracle.com>
>> Reviewed-by: Daniel Kiper <daniel.kiper at oracle.com>
>> ---
>> 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
>