[CRIU] [PATCH 1/5] crtools: preload libraries

[Andrew Vagin]

On Fri, Dec 06, 2013 at 12:42:45PM +0400, Pavel Emelyanov wrote:
> >>> @@ -0,0 +1,22 @@
> >>> +#ifndef __CR_PLUGIN_H__
> >>> +#define __CR_PLUGIN_H__
> >>> +
> >>> +#include "criu-plugin.h"
> >>> +
> >>> +struct cr_plugin_entry {
> >>> +	union {
> >>> +		cr_plugin_fini_t *cr_fini;
> >>> +	};
> >>> +
> >>> +	struct cr_plugin_entry *next;
> >>> +};
> >>> +
> >>> +struct cr_plugins {
> >>> +	struct cr_plugin_entry *cr_fini;
> >>> +};
> >>> +
> >>> +extern struct cr_plugins cr_plugins;
> >>
> >> Why should this info be public?
> > 
> > criu-plugin.h is public, this one isn't.
> 
> OK, s/public/criu-sources-visible/
> Why?

I see. will fix. Thanks.
> 
> >>
> >>> +
> >>> +void cr_lib_fini(void);
> >>> +int cr_lib_init(void);
> >>> +#endif
> >>> diff --git a/lib.c b/lib.c
> >>> new file mode 100644
> >>> index 0000000..387bb84
> >>> --- /dev/null
> >>> +++ b/lib.c
> >>> @@ -0,0 +1,123 @@
> >>> +#include <stdlib.h>
> >>> +#include <string.h>
> >>> +#include <dirent.h>
> >>> +#include <errno.h>
> >>> +#include <stdio.h>
> >>> +#include <dlfcn.h>
> >>> +
> >>> +#include "cr_options.h"
> >>> +#include "plugin.h"
> >>> +#include "log.h"
> >>> +#include "xmalloc.h"
> >>> +
> >>> +struct cr_plugins cr_plugins;
> >>> +
> >>> +static int cr_lib_load(char *path)
> >>> +{
> >>> +	struct cr_plugin_entry *ce;
> >>> +	cr_plugin_init_t *f_init;
> >>> +	cr_plugin_fini_t *f_fini;
> >>> +	void *h;
> >>> +
> >>> +	h = dlopen(path, RTLD_LAZY);
> >>> +	if (h == NULL) {
> >>> +		pr_err("Unable to load %s: %s", path, dlerror());
> >>> +		return -1;
> >>> +	}
> >>> +
> >>> +	ce = NULL;
> >>> +	f_fini = dlsym(h, "cr_plugin_fini");
> >>> +	if (f_fini) {
> >>> +		ce = xmalloc(sizeof(struct cr_plugin_entry));
> >>> +		if (ce == NULL)
> >>> +			return -1;
> >>> +		ce->cr_fini = f_fini;
> >>
> >> In patch #3 you use some fancy macro for this.
> > 
> > f_fini is a special case.
> >>
> >>> +	}
> >>> +
> >>> +	f_init = dlsym(h, "cr_plugin_init");
> >>> +	if (f_init && f_init()) {
> >>> +		xfree(ce);
> >>> +		return -1;
> >>> +	}
> >>> +
> >>> +	if (ce) {
> >>> +		ce->next = cr_plugins.cr_fini;
> >>> +		cr_plugins.cr_fini = ce;
> >>> +	}
> > 
> > because it's added in the list here
> 
> Why?

It should be added only if cr_plugin_init() doesn't fail.

I can rewrite this codee by using the macros, but I like this version
more.

> 
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +void cr_lib_fini(void)
> >>> +{
> >>> +	struct cr_plugin_entry *ce;
> >>> +
> >>> +	while (cr_plugins.cr_fini) {
> >>> +		ce = cr_plugins.cr_fini;
> >>> +		cr_plugins.cr_fini = cr_plugins.cr_fini->next;
> >>> +
> >>> +		ce->cr_fini();
> >>> +		xfree(ce);
> >>> +	}
> >>> +}
> >>> +
> >>> +int cr_lib_init(void)
> >>> +{
> >>> +	int exit_code = -1;
> >>> +	DIR *d;
> >>> +
> >>> +	memset(&cr_plugins, 0, sizeof(cr_plugins));
> >>> +
> >>> +	if (opts.libdir == NULL) {
> >>> +		char *path;
> >>> +
> >>> +		path = getenv("CRIU_LIBS_DIR");
> >>> +		if (path == NULL)
> >>
> >> I think we should declare some default library for it.
> > 
> > I thought about this. I don't know which path can be default and we can
> > add the default path in any moment.
> 
> Plz, declare the /var/lib/criu/ and use it.

Ok

> 
> > 
> >>
> >>> +			return 0;
> >>> +
> >>> +		opts.libdir = strdup(path);
> >>
> >> Why strdup?
> > 
> > becuase we need to copy the string, when we are parsing options,
> > so we are going to call free(opts.libdir)
> > 
> >>
> >>> +		if (opts.libdir == NULL) {
> >>> +			pr_perror("Can't allocate memory");
> >>> +			return -1;
> >>> +		}
> >>> +	}
> >>> +
> >>> +	d = opendir(opts.libdir);
> >>> +	if (d == 0) {
> >>> +		pr_perror("Unable to open directory %s", opts.libdir);
> >>> +		return -1;
> >>> +	}
> >>> +
> >>> +	while (1) {
> >>> +		char path[PATH_MAX];
> >>> +		struct dirent *de;
> >>> +		int len;
> >>> +
> >>> +		errno = 0;
> >>> +		de = readdir(d);
> >>> +		if (de == NULL) {
> >>> +			if (errno == 0)
> >>> +				break;
> >>> +			pr_perror("Unable to read the libraries directory");
> >>> +			goto err;
> >>> +		}
> >>> +
> >>> +		len = strlen(de->d_name);
> >>> +
> >>> +		if (len < 3 || strncmp(de->d_name + len - 3, ".so", 3))
> >>> +			continue;
> >>> +
> >>> +		snprintf(path, sizeof(path), "%s/%s", opts.libdir, de->d_name);
> >>> +
> >>> +		if (cr_lib_load(path))
> >>> +			goto err;
> >>> +	}
> >>> +
> >>> +	exit_code = 0;
> >>> +err:
> >>> +	closedir(d);
> >>> +
> >>> +	cr_lib_fini();
> >>> +
> >>> +	return exit_code;
> >>> +}
> >>>
> >>
> >>
> > .
> > 
> 
>