[PATCH v3 1/7] Initialize and setup eppic

[Atsushi Kumagai]

Hello Aravinda,

Thank you for updating.
I have two comments in this mail.


On Fri, 14 Dec 2012 14:56:04 +0530
Aravinda Prasad <aravinda at linux.vnet.ibm.com> wrote:

> This patch contains routines which initialize eppic and register call
> back function which will be called whenever a new eppic macro is loaded
> using eppic_load() API. The registered call back function executes the
> eppic macro as soon as it is loaded.
> 
> Signed-off-by: Aravinda Prasad <aravinda at linux.vnet.ibm.com>
> ---
>  Makefile          |    5 +++
>  extension_eppic.c |   84 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  extension_eppic.h |   23 +++++++++++++++
>  3 files changed, 111 insertions(+), 1 deletions(-)
>  create mode 100644 extension_eppic.c
>  create mode 100644 extension_eppic.h
> 
> diff --git a/Makefile b/Makefile
> index e6c4e5d..f123a5f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -69,7 +69,7 @@ $(OBJ_ARCH): $(SRC_ARCH)
>  	$(CC) $(CFLAGS_ARCH) -c -o ./$@ ./$(@:.o=.c) 
>  
>  makedumpfile: $(SRC) $(OBJ_PART) $(OBJ_ARCH)
> -	$(CC) $(CFLAGS) $(LDFLAGS) $(OBJ_PART) $(OBJ_ARCH) -o $@ $< $(LIBS)
> +	$(CC) $(CFLAGS) $(LDFLAGS) $(OBJ_PART) $(OBJ_ARCH) -rdynamic -o $@ $< $(LIBS)
>  	echo .TH MAKEDUMPFILE 8 \"$(DATE)\" \"makedumpfile v$(VERSION)\" \"Linux System Administrator\'s Manual\" > temp.8
>  	grep -v "^.TH MAKEDUMPFILE 8" makedumpfile.8 >> temp.8
>  	mv temp.8 makedumpfile.8
> @@ -79,6 +79,9 @@ makedumpfile: $(SRC) $(OBJ_PART) $(OBJ_ARCH)
>  	mv temp.5 makedumpfile.conf.5
>  	gzip -c ./makedumpfile.conf.5 > ./makedumpfile.conf.5.gz
>  
> +eppic_makedumpfile.so: extension_eppic.c
> +	$(CC) $(CFLAGS) -nostartfiles -shared -rdynamic -o $@ extension_eppic.c -fPIC -leppic
> +

First, I prepared eppic_makedumpfile.so with the code above, but I failed
with the message below:

  $ makedumpfile -cd31 -x vmlinux --eppic eppic_macro/mod.c vmcore dumpfile.cd31
  process_eppic_file: dlopen failed: /usr/lib64/eppic_makedumpfile.so: undefined symbol: setupterm
  
  makedumpfile Failed.
  $

so I think '-lncurses' should be specified as gcc option, or did I mistake ?


[...]
> +/* Initialize eppic */
> +int
> +_init()
> +{
> +	if (eppic_open() >= 0) {
> +
> +		/* Register call back functions */
> +		eppic_apiset(NULL, 3, sizeof(long), 0);
> +
> +		/* set the new function callback */
> +		eppic_setcallback(reg_callback);
> +
> +		return 0;
> +	}
> +	return 1;
> +}

Second, your code introduce _init() as constructor for dlopen().
I'm not sure about dlopen(), but I found the description which recommend
not to use _init() in man page:

   The obsolete symbols _init() and _fini()
       The linker recognizes special symbols _init and _fini.  If a dynamic library exports a routine named _init(), then that  code  is  executed
       after  the  loading,  before  dlopen()  returns.   If the dynamic library exports a routine named _fini(), then that routine is called just
       before the library is unloaded.  In case you need to avoid linking against the system startup files, this can be done by using  the  gcc(1)
       -nostartfiles command-line option.

       Using these routines, or the gcc -nostartfiles or -nostdlib options, is not recommended.  Their use may result in undesired behavior, since
       the constructor/destructor routines will not be executed (unless special measures are taken).

       Instead, libraries should export routines using the __attribute__((constructor)) and __attribute__((destructor)) function attributes.   See
       the  gcc  info pages for information on these.  Constructor routines are executed before dlopen() returns, and destructor routines are exe-
       cuted before dlclose() returns.

So, is it OK to continue to use _init() ?

BTW, would you send the patch to add the description of this feature
to README, man page and help message ?


Thanks
Atsushi Kumagai