[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [virt-tools-list] [libosinfo] Add osinfo_db_guess_os_from_location()



On Sun, Aug 21, 2011 at 09:57:41PM +0300, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak gnome org>
> 
> Add API to guess OS given an installation media location.
> ---
>  docs/reference/Libosinfo-sections.txt |    1 +
>  osinfo/libosinfo.syms                 |    1 +
>  osinfo/osinfo_db.c                    |  155 ++++++++++++++++++++++++++++++++-
>  osinfo/osinfo_db.h                    |    2 +
>  4 files changed, 158 insertions(+), 1 deletions(-)
> 
> diff --git a/docs/reference/Libosinfo-sections.txt b/docs/reference/Libosinfo-sections.txt
> index 0aee98f..80816e6 100644
> --- a/docs/reference/Libosinfo-sections.txt
> +++ b/docs/reference/Libosinfo-sections.txt
> @@ -18,6 +18,7 @@ osinfo_db_add_os
>  osinfo_db_add_platform
>  osinfo_db_add_device
>  osinfo_db_add_deployment
> +osinfo_db_guess_os_from_location
>  osinfo_db_unique_values_for_property_in_os
>  osinfo_db_unique_values_for_property_in_platform
>  osinfo_db_unique_values_for_property_in_device
> diff --git a/osinfo/libosinfo.syms b/osinfo/libosinfo.syms
> index aadcbc3..b318c9a 100644
> --- a/osinfo/libosinfo.syms
> +++ b/osinfo/libosinfo.syms
> @@ -15,6 +15,7 @@ LIBOSINFO_0.0.1 {
>  	osinfo_db_add_platform;
>  	osinfo_db_add_device;
>  	osinfo_db_add_deployment;
> +        osinfo_db_guess_os_from_location;
>  	osinfo_db_unique_values_for_property_in_os;
>  	osinfo_db_unique_values_for_property_in_platform;
>  	osinfo_db_unique_values_for_property_in_device;
> diff --git a/osinfo/osinfo_db.c b/osinfo/osinfo_db.c
> index 4c37365..9570cbd 100644
> --- a/osinfo/osinfo_db.c
> +++ b/osinfo/osinfo_db.c
> @@ -23,11 +23,43 @@
>   */
>  
>  #include <osinfo/osinfo.h>
> +#include <gio/gio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +#define MAX_VOLUME 32
> +#define MAX_SYSTEM 32
> +#define MAX_PUBLISHER 128
> +
> +#define PVD_OFFSET 0x00008000
> +#define BOOTABLE_TAG "EL TORITO SPECIFICATION"
> +
> +typedef struct _PrimaryVolumeDescriptor PrimaryVolumeDescriptor;
> +
> +struct _PrimaryVolumeDescriptor {
> +    guint8 ignored[8];
> +    gchar  system[MAX_SYSTEM];       /* System ID */
> +    gchar  volume[MAX_VOLUME];       /* Volume ID */
> +    guint8 ignored2[246];
> +    gchar  publisher[MAX_PUBLISHER]; /* Publisher ID */
> +    guint8 ignored3[1602];
> +};
> +
> +typedef struct _SupplementaryVolumeDescriptor SupplementaryVolumeDescriptor;
> +
> +struct _SupplementaryVolumeDescriptor {
> +    guint8 ignored[7];
> +    gchar  system[MAX_SYSTEM]; /* System ID */
> +};
>  
>  G_DEFINE_TYPE (OsinfoDb, osinfo_db, G_TYPE_OBJECT);
>  
>  #define OSINFO_DB_GET_PRIVATE(obj) (G_TYPE_INSTANCE_GET_PRIVATE ((obj), OSINFO_TYPE_DB, OsinfoDbPrivate))
>  
> +#define str_contains(str, substr) ((str) && \
> +                                   (substr) && \
> +                                   strstr((str), (substr)) != NULL)
> +
>  /**
>   * SECTION:osinfo_db
>   * @short_description: Database of all entities
> @@ -144,7 +176,6 @@ OsinfoOs *osinfo_db_get_os(OsinfoDb *db, const gchar *id)
>      return OSINFO_OS(osinfo_list_find_by_id(OSINFO_LIST(db->priv->oses), id));
>  }
>  
> -
>  /**
>   * osinfo_db_get_deployment:
>   * @db: the database
> @@ -316,6 +347,128 @@ void osinfo_db_add_deployment(OsinfoDb *db, OsinfoDeployment *deployment)
>      osinfo_list_add(OSINFO_LIST(db->priv->deployments), OSINFO_ENTITY(deployment));
>  }
>  
> +/**
> + * osinfo_db_guess_os_from_location:
> + * @db: the database
> + * @location: the location of an installation media
> + *
> + * Returns: (transfer none): the operating system, or NULL if guessing failed
> + */
> +OsinfoOs *osinfo_db_guess_os_from_location(OsinfoDb *db,
> +                                           const gchar *location)
> +{
> +    OsinfoOs *ret = NULL;
> +    PrimaryVolumeDescriptor pvd;
> +    SupplementaryVolumeDescriptor svd;
> +    GFile *file;
> +    GFileInputStream *stream;
> +    GError *error = NULL;
> +    GList *oss = NULL;
> +    GList *os_iter;
> +
> +    g_return_val_if_fail(OSINFO_IS_DB(db), NULL);
> +    g_return_val_if_fail(location != NULL, NULL);
> +
> +    file = g_file_new_for_commandline_arg(location);
> +    stream = g_file_read(file, NULL, &error);
> +    if (error != NULL) {
> +        g_warning ("failed to read %s: %s",
> +                   location,
> +                   error->message);
> +        g_error_free(error);
> +
> +        goto EXIT;
> +    }
> +
> +    bzero(&pvd, sizeof(pvd));
> +    if (g_input_stream_skip(G_INPUT_STREAM(stream),
> +                            PVD_OFFSET,
> +                            NULL,
> +                            &error) < 0) {
> +        g_warning ("failed to skip %d bytes in %s: %s",
> +                   PVD_OFFSET,
> +                   location,
> +                   error->message);
> +        g_error_free(error);
> +
> +        goto EXIT;
> +    }
> +
> +    if (g_input_stream_read(G_INPUT_STREAM(stream),
> +                            &pvd,
> +                            sizeof (pvd),
> +                            NULL,
> +                            &error) < 0) {
> +        g_warning ("failed to read primary volume descriptor from %s: %s",
> +                   location,
> +                   error->message);
> +        g_error_free(error);
> +
> +        goto EXIT;
> +    }

I was a little bit wary of you reading in the whole struct at once,
but it looks like all fields are at least 8 byte aligned, so we
don't need to worry about compiler padding.

> +
> +    pvd.volume[MAX_VOLUME - 1] = 0;
> +    pvd.system[MAX_SYSTEM - 1] = 0;
> +    pvd.publisher[MAX_PUBLISHER - 1] = 0;
> +
> +    if (pvd.volume[0] && (pvd.system[0] == 0 && pvd.publisher[0] == 0))
> +        goto EXIT;
> +
> +    bzero(&svd, sizeof(svd));

As mentioned on IRC, memset() is a little better.

> +    if (g_input_stream_read(G_INPUT_STREAM(stream),
> +                            &svd,
> +                            sizeof(svd),
> +                            NULL,
> +                            &error) < 0) {
> +        g_warning ("failed to read supplementary volume descriptor from %s: %s",
> +                   location,
> +                   error->message);
> +        g_error_free(error);
> +
> +        goto EXIT;
> +    }

You need to have

     svd.system[MAX_SYSTEM - 1] = '\0';

> +
> +    if (strncmp(BOOTABLE_TAG, svd.system, sizeof(BOOTABLE_TAG) != 0)) {
> +        g_warning ("%s not a bootable media", location);
> +
> +        goto EXIT;
> +    }

Otherwise this is at risk of array out of bounds access.

> +    oss = osinfo_list_get_elements(OSINFO_LIST(db->priv->oses));
> +    for (os_iter = oss; os_iter; os_iter = os_iter->next) {
> +        OsinfoOs *os = OSINFO_OS(os_iter->data);
> +        OsinfoMediaList *media_list = osinfo_os_get_media_list(os);
> +        GList *medias = osinfo_list_get_elements(OSINFO_LIST(media_list));
> +        GList *media_iter;
> +
> +        for (media_iter = medias; media_iter; media_iter = media_iter->next) {
> +            OsinfoMedia *media = OSINFO_MEDIA(media_iter->data);
> +            const gchar *media_volume = osinfo_media_get_volume_id(media);
> +            const gchar *media_system = osinfo_media_get_system_id(media);
> +            const gchar *media_publisher = osinfo_media_get_publisher_id(media);
> +
> +            if (str_contains(pvd.volume, media_volume) &&
> +                (str_contains(pvd.system, media_system) ||
> +                 str_contains(pvd.publisher, media_publisher))) {
> +                ret = os;
> +                break;
> +            }
> +        }
> +
> +        g_list_free(medias);
> +        g_object_unref(media_list);
> +
> +        if (ret)
> +            break;
> +    }
> +
> +EXIT:
> +    g_list_free(oss);
> +    g_object_unref (stream);
> +    g_object_unref (file);
> +
> +    return ret;
> +}
>  
>  struct osinfo_db_populate_values_args {
>      GHashTable *values;
> diff --git a/osinfo/osinfo_db.h b/osinfo/osinfo_db.h
> index c9d506e..ca2a83e 100644
> --- a/osinfo/osinfo_db.h
> +++ b/osinfo/osinfo_db.h
> @@ -92,6 +92,8 @@ void osinfo_db_add_platform(OsinfoDb *db, OsinfoPlatform *platform);
>  void osinfo_db_add_device(OsinfoDb *db, OsinfoDevice *device);
>  void osinfo_db_add_deployment(OsinfoDb *db, OsinfoDeployment *deployment);
>  
> +OsinfoOs *osinfo_db_guess_os_from_location(OsinfoDb *db, const gchar *location);
> +
>  // Get me all unique values for property "vendor" among operating systems
>  GList *osinfo_db_unique_values_for_property_in_os(OsinfoDb *db, const gchar *propName);


ACK if those small changes are made

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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]