Hey, Looks good, a few cosmetic comments below, On Thu, Sep 13, 2012 at 04:04:29PM +0200, Michal Privoznik wrote:
In some cases telling OS version is redundant as ISO image with specified OS is passed some arguments later as disk. Don't require --os then. --- examples/virtxml.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 105 insertions(+), 5 deletions(-)
diff --git a/examples/virtxml.c b/examples/virtxml.c index b0c3a77..4ec9cd8 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -293,6 +293,98 @@ add_iface_str(const gchar *option_name, return TRUE; }
+static OsinfoOs * +find_os(const gchar *os_str) +{ + OsinfoDb *db = get_default_osinfo_db(); + OsinfoOs *ret = NULL; + + if (!db) + return NULL; + + ret = osinfo_db_get_os(db, os_str); + + return ret; +} + +static OsinfoOs * +find_os_by_short_id(const char *short_id) +{ + OsinfoDb *db = get_default_osinfo_db(); + OsinfoOs *ret = NULL; + OsinfoOsList *oses = NULL; + GList *list, *list_iterator;
I generally go with the much shorter 'it' instead of list_iterator, no need to change if you prefer the more explicit version
+ + if (!db) + return NULL; + + oses = osinfo_db_get_os_list(db); + + if (!oses) + goto cleanup; + + list = osinfo_list_get_elements(OSINFO_LIST(oses)); + for (list_iterator = list; list_iterator; list_iterator = list_iterator->next) { + const char *id = osinfo_entity_get_param_value(list_iterator->data, + OSINFO_PRODUCT_PROP_SHORT_ID); + + if (id && g_str_equal(id, short_id)) {
you can use g_strcmp0 here to avoid testing 'id'.
+ ret = OSINFO_OS(list_iterator->data); + g_object_ref(ret); + break; + } + } + + g_object_unref(oses); + +cleanup: + return ret; +} + +static OsinfoOs * +guess_os_from_disk(GList *disk_list) +{ + OsinfoOs *ret = NULL; + GList *tmp = g_list_first(disk_list); + OsinfoLoader *loader = osinfo_loader_new(); + OsinfoDb *db; + + osinfo_loader_process_default_path(loader, NULL); + db = osinfo_loader_get_db(loader); + + while (tmp) {
I much prefer the for loop you use for a similar purpose in find_os_by_short_id, especially with the use of 'continue' in this loop.
+ char *path = (char *) tmp->data; + char *sep = strchr(path, ','); + OsinfoMedia *media = NULL; + OsinfoMedia *matched_media = NULL; + + if (sep) { + path = g_strndup(path, sep-path); + }
the {} with the 1 line block are inconsistent with what is done in the rest of the patch. Christophe