[libvirt] [PATCH 1/9] Add helpers for resolving path to resources in build tree

Nehal J Wani nehaljw.kkd1 at gmail.com
Thu Apr 24 19:21:54 UTC 2014


> + * virFileFindResourceFull:
> + * @filename: libvirt distributed filename without any path
> + * @prefix: optional string to prepend to filename
> + * @suffix: optional string to append to filename
> + * @builddir: location of the binary in the source tree build tree
> + * @installdir: location of the installed binary
> + * @envname: environment variable used to override all dirs
> + *
> + * A helper which will return a path to @filename within
> + * the current build tree, if the calling binary is being
> + * run from the source tree. Otherwise it will return the
> + * path in the installed location.
> + *
> + * If @envname is none-NULL it will override all other
> + * directory lookup
> + *
> + * Only use this with @filename files that are part of
> + * the libvirt tree, not 3rd party binaries/files.
> + */

Shouldn't we be mentioning here that its caller's responsibility to
free the returned value?

> +char *virFileFindResourceFull(const char *filename,
> +                              const char *prefix,
> +                              const char *suffix,
> +                              const char *builddir,
> +                              const char *installdir,
> +                              const char *envname)
> +{


>  char *virFindFileInPath(const char *file);
>
> +char *virFileFindResource(const char *filename,
> +                          const char *builddir,
> +                          const char *installdir);
> +char *virFileFindResourceFull(const char *filename,
> +                              const char *prefix,
> +                              const char *suffix,
> +                              const char *builddir,
> +                              const char *installdir,
> +                              const char *envname);
> +void virFileActivateDirOverride(const char *argv0);
> +

I really liked the generalization. I have gone through each patch of
this series and have only one question. The recursive grep:
grep "main(int argc" -r ./*
in libvirt's root directory, gives the following files:
./daemon/libvirtd.c ./examples/domsuspend/suspend.c
./examples/hellolibvirt/hellolibvirt.c ./examples/openauth/openauth.c
./examples/object-events/event-test.c ./src/lxc/lxc_controller.c
./src/security/virt-aa-helper.c ./src/locking/lock_daemon.c
./src/locking/sanlock_helper.c ./src/util/iohelper.c
./src/storage/parthelper.c ./tests/test_conf.c ./tests/testutils.h
./tests/testutils.h ./tests/commandhelper.c ./tests/shunloadtest.c
./tests/ssh.c ./tests/seclabeltest.c ./tools/virt-host-validate.c
./tools/virt-login-shell.c ./tools/virsh.c
Shouldn't we be calling virFileActivateDirOverride(argv[0]) in all of them?

Regards,
Nehal J Wani




More information about the libvir-list mailing list