[libvirt] [PATCHv2 1/6] Add utility functions for storing uninstalled location
Eric Blake
eblake at redhat.com
Wed Apr 23 20:21:26 UTC 2014
On 03/25/2014 02:23 AM, Nehal J Wani wrote:
> src/libvirt_private.syms
> *Add symbols
>
> daemon/libvirtd.c
> *Set uninstallDir when libvirtd is running uninstalled
> from a build tree
>
> src/util/virutil.c
> *Introduce virSetUninstalledDir
> *Introduce virGetUninstalledDir
>
> ---
> daemon/libvirtd.c | 1 +
> src/libvirt_private.syms | 2 ++
> src/util/virutil.c | 27 +++++++++++++++++++++++++++
> src/util/virutil.h | 3 +++
> 4 files changed, 33 insertions(+), 0 deletions(-)
>
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 4179147..dc3da2a 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -1165,6 +1165,7 @@ int main(int argc, char **argv) {
> exit(EXIT_FAILURE);
> }
> *tmp = '\0';
> + virSetUninstalledDir(argv[0]);
Given Dan's complaint about us abusing this variable, I'll squash in
something to make the naming more obvious.
> +++ b/src/util/virutil.c
> @@ -2204,3 +2204,30 @@ void virUpdateSelfLastChanged(const char *path)
> selfLastChanged = sb.st_ctime;
> }
> }
> +
> +static char *uninstalledDir = NULL;
Static variables are guaranteed 0-initialization by the C standard;
explicitly assigning NULL makes some compilers actually bloat the
executable size because they stick the variable into .data instead of
.bss (gcc knows better, though).
> +/**
Style - we've been using 2 blank lines between functions (not enforced,
but does make it visually easier to spot breaks when scrolling through
files).
> {
> ignore_value(VIR_STRDUP(uninstalledDir, path));
Memory leak if this gets called more than once (not that it does; we
call it 0 times when installed and exactly once when uninstalled) -
plugging it will make sure we don't get false positives from Coverity
and the like.
> +/**
> + * virGetUninstalledDir:
> + *
> + * If libvirtd (or other binary) is running uninstalled from a build tree,
> + * return the directory containing the binary. A NULL return implies that
> + * the binary has been installed and paths are relying on <configmake.h>
> + * locations.
> + */
> +char *virGetUninstalledDir(void)
Needs a const.
Potential ACK. Here's what I'll squash in, and I'll push the result
after reviewing the rest of the series if I don't spot anything else major.
Hmm, in re-reading things, I noticed that the one place you are calling
the new set function, you are passing in /path/to/builddir/daemon/.libs/
as the directory. I think it might work better to pass in
/path/to/builddir, and teach all clients to specify paths directly
relative to builddir, than having to do things relative to
/path/to/builddir/daemon/.libs/../..; I'll have to see how you actually
use the directory in later patches.
diff --git i/daemon/libvirtd.c w/daemon/libvirtd.c
index f0faf1a..0754fb5 100644
--- i/daemon/libvirtd.c
+++ w/daemon/libvirtd.c
@@ -1158,23 +1158,26 @@ int main(int argc, char **argv) {
if (strstr(argv[0], "lt-libvirtd") ||
strstr(argv[0], "/daemon/.libs/libvirtd")) {
+ char *dir;
char *tmp = strrchr(argv[0], '/');
char *cpumap;
- if (!tmp) {
- fprintf(stderr, _("%s: cannot identify driver
directory\n"), argv[0]);
+ char *driverdir;
+
+ if (!tmp || VIR_STRNDUP(dir, argv[0], tmp - argv[0]) < 0) {
+ fprintf(stderr, _("%s: cannot identify driver directory\n"),
+ argv[0]);
exit(EXIT_FAILURE);
}
- *tmp = '\0';
- virSetUninstalledDir(argv[0]);
- char *driverdir;
- if (virAsprintfQuiet(&driverdir, "%s/../../src/.libs", argv[0])
< 0 ||
+ virSetUninstalledDir(dir);
+ if (virAsprintfQuiet(&driverdir, "%s/../../src/.libs", dir) < 0 ||
virAsprintfQuiet(&cpumap, "%s/../../src/cpu/cpu_map.xml",
- argv[0]) < 0) {
+ dir) < 0) {
fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
exit(EXIT_FAILURE);
}
if (access(driverdir, R_OK) < 0) {
- fprintf(stderr, _("%s: expected driver directory '%s' is
missing\n"),
+ fprintf(stderr,
+ _("%s: expected driver directory '%s' is missing\n"),
argv[0], driverdir);
exit(EXIT_FAILURE);
}
@@ -1184,7 +1187,7 @@ int main(int argc, char **argv) {
#endif
cpuMapOverride(cpumap);
VIR_FREE(cpumap);
- *tmp = '/';
+ VIR_FREE(dir);
/* Must not free 'driverdir' - it is still used */
}
diff --git i/src/util/virutil.c w/src/util/virutil.c
index 244b9e9..dabd4f8 100644
--- i/src/util/virutil.c
+++ w/src/util/virutil.c
@@ -2205,7 +2205,9 @@ void virUpdateSelfLastChanged(const char *path)
}
}
-static char *uninstalledDir = NULL;
+
+static char *uninstalledDir;
+
/**
* virSetUninstalledDir:
* @path: directory containing an uninstalled binary, for use in locating
@@ -2214,8 +2216,9 @@ static char *uninstalledDir = NULL;
* Set a pointer to the path which can be accessed by all other APIs using
* virGetUninstalledDir().
*/
-void virSetUninstalledDir(char *path)
+void virSetUninstalledDir(const char *path)
{
+ VIR_FREE(uninstalledDir);
ignore_value(VIR_STRDUP(uninstalledDir, path));
}
@@ -2227,7 +2230,7 @@ void virSetUninstalledDir(char *path)
* the binary has been installed and paths are relying on <configmake.h>
* locations.
*/
-char *virGetUninstalledDir(void)
+const char *virGetUninstalledDir(void)
{
return uninstalledDir;
}
diff --git i/src/util/virutil.h w/src/util/virutil.h
index 3e6ba47..437167a 100644
--- i/src/util/virutil.h
+++ w/src/util/virutil.h
@@ -202,7 +202,7 @@ bool virIsSUID(void);
time_t virGetSelfLastChanged(void);
void virUpdateSelfLastChanged(const char *path);
-void virSetUninstalledDir(char *path);
-char *virGetUninstalledDir(void);
+void virSetUninstalledDir(const char *path);
+const char *virGetUninstalledDir(void);
#endif /* __VIR_UTIL_H__ */
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140423/e2e663eb/attachment-0001.sig>
More information about the libvir-list
mailing list