On Wed, Jul 06, 2016 at 01:28:31PM +0200, Fabiano Fidêncio wrote:
> On Tue, Jun 28, 2016 at 5:52 PM, Daniel P. Berrange <berrange@xxxxxxxxxx> wrote:
> > With the split out database, the RNG schema will no longer
> > be installed by the code, instead it will be distributed
> > as part of the database. Thus we should look in the database
> > locations to find the schema.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> > ---
> > po/POTFILES.in | 1 +
> > tools/Makefile.am | 2 +-
> > tools/osinfo-db-util.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
> > tools/osinfo-db-util.h | 7 +++++++
> > tools/osinfo-db-validate.c | 29 ++++++++++++++++++---------
> > 5 files changed, 78 insertions(+), 10 deletions(-)
[snip]
> > + for (i = 0; i < npaths; i++) {
> > + ret = g_file_resolve_relative_path(paths[i], file);
> > + if (g_file_query_exists(ret, NULL))
> > + break;
> > + g_object_unref(ret);
> > + ret = NULL;
> > + }
> > +
> > + if (!ret) {
>
> This is just my personal preference, but I really would like to have
> explicit comparisons in all cases that you're not checking against a
> boolean. But feel free to ignore, probably there are a lot of other
> places doing the same.
IMHO this is pretty standard style for checking pointers for NULL
status.
> > + g_set_error(err, 0, 0,
> > + _("Unable to locate '%s' in any database location"),
> > + file);
> > + return NULL;
>
> I would not use "return NULL" here. ret is NULL at this point and
> returning ret would be okay.
Yep.
>
> > + }
> > +
> > + return ret;
> > +}
> > +
Regards,
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 :|
_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo
