On Wed, Mar 14, 2012 at 02:24:18PM +0200, Zeeshan Ali (Khattak) wrote:
> On Tue, Mar 13, 2012 at 6:32 PM, Daniel P. Berrange <berrange@xxxxxxxxxx> wrote:
> > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> ACK to both patches otherwise, one comment only to this one:
>
> > index 337f737..b009323 100644
> > --- a/osinfo/osinfo_loader.c
> > +++ b/osinfo/osinfo_loader.c
> > @@ -1271,11 +1271,14 @@ void osinfo_loader_process_default_path(OsinfoLoader *loader,
> > GError **err)
> > {
> > GFile *file;
> > + gchar *dbdir;
> > const gchar *path = getenv("OSINFO_DATA_DIR");
> > if (!path)
> > - path = DATA_DIR;
> > + path = PKG_DATA_DIR;
> >
> > - file = g_file_new_for_path(path);
> > + dbdir = g_strdup_printf("%s/db", path);
>
> Why not just pass the whole path from CFLAGS just like DATA_DIR?
We have to do the g_strdup_printf() bit for when 'path' comes
from an env variable, so it is simpler todo the same for the
built-in default too. That way later code does not have worry
about whether 'dbdir' is a constant string or a heap allocated
string.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
