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

Re: [virt-tools-list] [libosinfo 2/2] Correct & further restrict media matching logic



On Thu, Feb 23, 2012 at 02:00:42AM +0200, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak gnome org>
> 
> Recently we started to use application IDs for detection, which is
> itself a good change but we ended-up turning the matching logic a lot
> more liberal than it should be. This patches fixes that and makes the
> application ID match compulsory.
> ---
>  osinfo/osinfo_db.c |    9 ++++-----
>  1 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/osinfo/osinfo_db.c b/osinfo/osinfo_db.c
> index ecc8fbd..d1eba9f 100644
> --- a/osinfo/osinfo_db.c
> +++ b/osinfo/osinfo_db.c
> @@ -386,11 +386,10 @@ OsinfoOs *osinfo_db_guess_os_from_media(OsinfoDb *db,
>              const gchar *os_publisher = osinfo_media_get_publisher_id(os_media);
>              const gchar *os_application = osinfo_media_get_application_id(os_media);
>  
> -            if ((match_regex (os_volume, media_volume) ||
> -                 match_regex (os_application, media_application))
> -                 &&
> -                (match_regex (os_system, media_system) ||
> -                 match_regex (os_publisher, media_publisher))) {
> +            if (match_regex (os_volume, media_volume) &&
> +                match_regex (os_system, media_system) &&
> +                match_regex (os_application, media_application) &&
> +                match_regex (os_publisher, media_publisher)) {
>                  ret = os;
>                  if (matched_media != NULL)
>                      *matched_media = os_media;

This is going in the right direction, but IMHO it is not complete. With
this, it is neccessary to fully specify all fields in the OSinfo database
even if they are not really neccessary for a proper match. IMHO we need
to also fix the 'match_regex' macro logic.

With the alternative patch I just sent, then we don't need to add in all
that CDIMAGE* stuff in your first patch.

Regards,
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]