[Libosinfo] [libosinfo PATCH 1/3] tree: Add osinfo_tree_has_treeinfo()

[Fabiano Fidêncio]

On Mon, Dec 10, 2018 at 1:46 PM Daniel P. Berrangé <berrange at redhat.com> wrote:
>
> On Mon, Dec 10, 2018 at 01:37:35PM +0100, Fabiano Fidêncio wrote:
> > As not all the "trees" we stored in osinfo-db have "treeinfo" data,
> > let's add a new method, osinfo_tree_has_treeinfo(), that can be used to
> > check whether the tree has treeinfo or not.
> >
> > Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> > ---
> >  osinfo/libosinfo.syms  |  2 ++
> >  osinfo/osinfo_loader.c | 25 +++++++++++++++++--------
> >  osinfo/osinfo_tree.c   | 39 +++++++++++++++++++++++++++++++++++++++
> >  osinfo/osinfo_tree.h   |  3 +++
> >  4 files changed, 61 insertions(+), 8 deletions(-)
> >
> > diff --git a/osinfo/libosinfo.syms b/osinfo/libosinfo.syms
> > index 39906c4..d87b7c0 100644
> > --- a/osinfo/libosinfo.syms
> > +++ b/osinfo/libosinfo.syms
> > @@ -548,6 +548,8 @@ LIBOSINFO_1.3.0 {
> >       osinfo_os_get_all_device_links;
> >       osinfo_os_get_image_list;
> >       osinfo_os_get_maximum_resources;
> > +
> > +     osinfo_tree_has_treeinfo;
> >  } LIBOSINFO_0.2.13;
> >
> >  /* Symbols in next release...
> > diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
> > index 17cb563..cea5070 100644
> > --- a/osinfo/osinfo_loader.c
> > +++ b/osinfo/osinfo_loader.c
> > @@ -1195,7 +1195,7 @@ static OsinfoTree *osinfo_loader_tree(OsinfoLoader *loader,
> >  {
> >      xmlNodePtr *nodes = NULL;
> >      guint i;
> > -
> > +    gboolean has_treeinfo = FALSE;
> >      gchar *arch = (gchar *)xmlGetProp(root, BAD_CAST "arch");
> >      const OsinfoEntityKey keys[] = {
> >          { OSINFO_TREE_PROP_URL, G_TYPE_STRING },
> > @@ -1222,27 +1222,36 @@ static OsinfoTree *osinfo_loader_tree(OsinfoLoader *loader,
> >              continue;
> >
> >          if (g_str_equal((const gchar *)nodes[i]->name,
> > -                        OSINFO_TREE_PROP_TREEINFO_FAMILY + strlen("treeinfo-")))
> > +                        OSINFO_TREE_PROP_TREEINFO_FAMILY + strlen("treeinfo-"))) {
> >              osinfo_entity_set_param(OSINFO_ENTITY(tree),
> >                                      OSINFO_TREE_PROP_TREEINFO_FAMILY,
> >                                      (const gchar *)nodes[i]->children->content);
> > -        else if (g_str_equal((const gchar *)nodes[i]->name,
> > -                             OSINFO_TREE_PROP_TREEINFO_VARIANT + strlen("treeinfo-")))
> > +            has_treeinfo = TRUE;
> > +        } else if (g_str_equal((const gchar *)nodes[i]->name,
> > +                             OSINFO_TREE_PROP_TREEINFO_VARIANT + strlen("treeinfo-"))) {
> >              osinfo_entity_set_param(OSINFO_ENTITY(tree),
> >                                      OSINFO_TREE_PROP_TREEINFO_VARIANT,
> >                                      (const gchar *)nodes[i]->children->content);
> > -        else if (g_str_equal((const gchar *)nodes[i]->name,
> > -                             OSINFO_TREE_PROP_TREEINFO_VERSION + strlen("treeinfo-")))
> > +            has_treeinfo = TRUE;
> > +        } else if (g_str_equal((const gchar *)nodes[i]->name,
> > +                             OSINFO_TREE_PROP_TREEINFO_VERSION + strlen("treeinfo-"))) {
> >              osinfo_entity_set_param(OSINFO_ENTITY(tree),
> >                                      OSINFO_TREE_PROP_TREEINFO_VERSION,
> >                                      (const gchar *)nodes[i]->children->content);
> > -        else if (g_str_equal((const gchar *)nodes[i]->name,
> > -                             OSINFO_TREE_PROP_TREEINFO_ARCH + strlen("treeinfo-")))
> > +            has_treeinfo = TRUE;
> > +        } else if (g_str_equal((const gchar *)nodes[i]->name,
> > +                             OSINFO_TREE_PROP_TREEINFO_ARCH + strlen("treeinfo-"))) {
> >              osinfo_entity_set_param(OSINFO_ENTITY(tree),
> >                                      OSINFO_TREE_PROP_TREEINFO_ARCH,
> >                                      (const gchar *)nodes[i]->children->content);
> > +            has_treeinfo = TRUE;
> > +        }
> >      }
>
> None of this is needed, when you can just do:
>
>
>    if (nnodes) {
>       ... set has_treeinfo...
>
> because nnodes > 0, if-and-only-if,  there were child nodes
> under <treeinfo>...

I thought about that but the check done inside the loop made me a
little bit worried of just relying on the number of nodes.
1220         if (!nodes[i]->children ||
1221             nodes[i]->children->type != XML_TEXT_NODE)
1222             continue;

Changes are done locally!