Created attachment 223324 [details] [review]
os-database: avoid crash during wizard if loading failed

Review of attachment 223324 [details] [review]:

As I understand it, libosinfo will return a NULL database if there's an error parsing any of the databases files (or at least if there's an error parsing the additional file we're providing). Imo it should be made more robust wrt that, but ACK to your patch anyway, better to be robust when something unexpected happens.

::: src/os-database.vala
@@ +39,3 @@
                                                   out Media os_media,
                                                   Cancellable? cancellable) throws GLib.Error {
+        os_media = null;

Is it valid to set this to null while it's declared as "out Media os_media" ?

(In reply to comment #2)
> Review of attachment 223324 [details] [review]:
> 
> As I understand it, libosinfo will return a NULL database if there's an error
> parsing any of the databases files (or at least if there's an error parsing the
> additional file we're providing). Imo it should be made more robust wrt that,
> but ACK to your patch anyway, better to be robust when something unexpected
> happens.
> 
> ::: src/os-database.vala
> @@ +39,3 @@
>                                                    out Media os_media,
>                                                    Cancellable? cancellable)
> throws GLib.Error {
> +        os_media = null;
> 
> Is it valid to set this to null while it's declared as "out Media os_media" ?

I think so, but I will change it to out Media? instead.

Actually, iirc, out Media? means you can pass null as argument to the function, not that the out value can be null. All out arguments can return null. So I won't change it

Attachment 223324 [details] pushed as 9748749 - os-database: avoid crash during wizard if loading failed

Created attachment 223434 [details] [review]
osinfo: ignore db loading errors

When reading libosinfo OS database, errors will get reported when a
file is invalid, missing, ... When this happens, we currently
give up on using libosinfo. This commit catches osinfo DB load errors,
but make sure we still try to use the libosinfo DB. At worse it will
be empty, but in most cases it should give us access to a partial
DB.

This new patch should give us better behaviour when falling back.

Review of attachment 223434 [details] [review]:

ack

Attachment 223434 [details] pushed as f3583de - osinfo: ignore db loading errors