On Thu, Feb 14, 2019 at 10:57 AM Christophe Fergeau <cferg...@redhat.com> wrote:
> Hey, > > On Wed, Feb 13, 2019 at 08:19:25PM +0100, Fabiano Fidêncio wrote: > > Identifiers as volume-id, application, publisher, and system are not > > null terminated and cannot be assumed as so. > > > > By assuming those are null terminated strings, libosinfo ends up not > > counting the last character of a MAX_* string and, consequently, not > > properly identifying medias that have their identifiers with the MAX_* > > size. > > > > One example is the ubuntu-18.04.1.0-live-server-amd64.iso media, which > > has as volume-id 'Ubuntu-Server 18.04.1+ LTS amd64'. As the volume-id > > has exactly 32 characters it's never been matched as when reading the > > media's volume-id it'd be read as 'Ubuntu-Server 18.04.1+ LTS amd6'. > > I don't think we have any test case for that code, do we? > We don't have any specific test case for this code. But the whole test-isodetect relies on this (and that's the way I found out the issue when adding the new test data). Do you want a specific test for this or are you fine with the patch 4 covering this case? > > > --- > > osinfo/osinfo_media.c | 37 +++++++++++++++++++++++-------------- > > 1 file changed, 23 insertions(+), 14 deletions(-) > > > > diff --git a/osinfo/osinfo_media.c b/osinfo/osinfo_media.c > > index 9f77504..eaf67e2 100644 > > --- a/osinfo/osinfo_media.c > > +++ b/osinfo/osinfo_media.c > > @@ -137,6 +137,11 @@ struct _CreateFromLocationAsyncData { > > > > gsize offset; > > gsize length; > > + > > + gchar *volume; > > + gchar *system; > > + gchar *application; > > + gchar *publisher; > > }; > > > > static void create_from_location_async_data_free > > @@ -144,6 +149,10 @@ static void create_from_location_async_data_free > > { > > g_object_unref(data->file); > > g_object_unref(data->res); > > + g_free(data->volume); > > + g_free(data->system); > > + g_free(data->application); > > + g_free(data->publisher); > > > > g_slice_free(CreateFromLocationAsyncData, data); > > } > > @@ -809,18 +818,18 @@ > create_from_location_async_data(CreateFromLocationAsyncData *data) > > OSINFO_MEDIA_PROP_URL, > > uri); > > g_free(uri); > > - if (!is_str_empty(data->pvd.volume)) > > + if (!is_str_empty(data->volume)) > > osinfo_entity_set_param(OSINFO_ENTITY(media), > > OSINFO_MEDIA_PROP_VOLUME_ID, > > - data->pvd.volume); > > - if (!is_str_empty(data->pvd.system)) > > + data->volume); > > + if (!is_str_empty(data->system)) > > osinfo_entity_set_param(OSINFO_ENTITY(media), > > OSINFO_MEDIA_PROP_SYSTEM_ID, > > - data->pvd.system); > > - if (!is_str_empty(data->pvd.publisher)) > > + data->system); > > + if (!is_str_empty(data->publisher)) > > osinfo_entity_set_param(OSINFO_ENTITY(media), > > OSINFO_MEDIA_PROP_PUBLISHER_ID, > > - data->pvd.publisher); > > + data->publisher); > > if (!is_str_empty(data->pvd.application)) > > osinfo_entity_set_param(OSINFO_ENTITY(media), > > OSINFO_MEDIA_PROP_APPLICATION_ID, > > @@ -1159,19 +1168,19 @@ static void on_pvd_read(GObject *source, > > return; > > } > > > > - data->pvd.volume[MAX_VOLUME - 1] = 0; > > - g_strchomp(data->pvd.volume); > > + data->volume = g_strndup(data->pvd.volume, MAX_VOLUME); > > + g_strchomp(data->volume); > > > > - data->pvd.system[MAX_SYSTEM - 1] = 0; > > - g_strchomp(data->pvd.system); > > + data->system = g_strndup(data->pvd.system, MAX_SYSTEM); > > + g_strchomp(data->system); > > > > - data->pvd.publisher[MAX_PUBLISHER - 1] = 0; > > - g_strchomp(data->pvd.publisher); > > + data->publisher = g_strndup(data->pvd.publisher, MAX_PUBLISHER); > > + g_strchomp(data->publisher); > > > > - data->pvd.application[MAX_APPLICATION - 1] = 0; > > + data->application = g_strndup(data->pvd.application, > MAX_APPLICATION); > > g_strchomp(data->pvd.application); > > This should be g_strchomp(data->application); > Fixed. > > Reviewed-by: Christophe Fergeau <cferg...@redhat.com> > > Christophe >
_______________________________________________ Libosinfo mailing list Libosinfo@redhat.com https://www.redhat.com/mailman/listinfo/libosinfo