hughsie/libxmlb

Add a way to return an empty resultset without using G_IO_ERROR_NOT_FOUND

Opened this issue · 7 comments

Currently, libxmlb queries will set the error G_IO_ERROR_NOT_FOUND if they find no matching elements. A lot of queries will often return no results (that’s just the way XML is), so this code path is hit often. If a lot of queries are run quickly (for example, when loading gnome-software), the cost of allocating a GError to indicate an empty resultset becomes measurable.

It would be nice if libxmlb would provide an alternate codepath which indicates an empty resultset some other way, without needing an allocation.

This might be possible without any new API: the documentation for the xb_silo_query() functions already says they return NULL if unfound, so callers should already be handling that. If that’s acceptable, we could just basically remove the g_set_error (G_IO_ERROR_NOT_FOUND) calls.

See https://gitlab.gnome.org/GNOME/gnome-software/-/merge_requests/1749 for some more context.

FWIW, I think Christians patch is probably the right thing to do; it;s highly unlikely the images = xb_node_query (screenshot, "image", 0, NULL); call is invalid XPath... :) -- is it possible to statically allocate a GError for "not found" and then unref it? g_error_unref would have been wonderful...

it;s highly unlikely the images = xb_node_query (screenshot, "image", 0, NULL); call is invalid XPath... :)

Sure, but we do have much more complex queries than that in gnome-software, and some of them have been broken before.

is it possible to statically allocate a GError for "not found" and then unref it? g_error_unref would have been wonderful...

Nope, I’ve tried hacks like that a few times over the years and it never works out well.

So the idea would be to use a in-xmlb ref'd GPtrArray to return "no results, but a valid query"? If so, that makes sense to me, but I think we'd need to make it opt-in somehow. Is there anything in XPath we can [ab]use?

Did you see this suggestion in my original post? 👇

This might be possible without any new API: the documentation for the xb_silo_query() functions already says they return NULL if unfound, so callers should already be handling that.

Is that not correct? If it is correct, callers should already be expecting to handle NULL for ‘empty resultset’, so there shouldn’t need to be an opt-in for the changes.

Right, but if we start returning NULL without setting GError lots of things are going to start exploding...

But the documentation for xb_silo_query() already says Returns: …, or %NULL if unfound. Callers should already be expecting NULL.

I’m reading that documentation as saying it can return NULL without setting a GError. Normally the NULL return value isn’t specified for error conditions.

Are you saying it should be read the other way — that NULL can only be returned if an error is set? In that case, we could go with your static-and-empty GPtrArray suggestion. That wouldn’t have to be opt-in unless you think code is currently expecting there to always be at least one result in the returned GPtrArray if it’s non-NULL.

Are you saying it should be read the other way — that NULL can only be returned if an error is set

Yup -- exactly that.

That wouldn’t have to be opt-in

I've got a nagging feeling that fwupd relies on ==NULL for no-results.