Re: [PATCH 3/4] Validate the bios_date format for <sysinfo>

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On 04/30/2013 08:19 PM, John Ferlan wrote:
> ---
>  src/conf/domain_conf.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a8b5dfd..43273f8 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11591,6 +11591,30 @@ virDomainDefParseXML(xmlDocPtr xml,
>                  goto error;
>              }
>          }
> +        if (def->sysinfo->bios_date != NULL) {
> +            char *date = def->sysinfo->bios_date;
> +            char *ptr;
> +            struct tm tm;
> +            memset(&tm, 0, sizeof(tm));
> +
> +            /* Validate just the format of the date
> +             * Expect mm/dd/yyyy or mm/dd/yy,
> +             * where yy must be 00->99 and would be assumed to be 19xx
> +             * a yyyy date should be 1900 and beyond
> +             */
> +            if (virStrToLong_i(date, &ptr, 10, &tm.tm_mon) < 0 ||
> +                *ptr != '/' ||
> +                virStrToLong_i(ptr+1, &ptr, 10, &tm.tm_mday) < 0 ||
> +                *ptr != '/' ||
> +                virStrToLong_i(ptr+1, &ptr, 10, &tm.tm_year) < 0 ||
> +                *ptr != '\0' ||
> +                (tm.tm_year < 0 || (tm.tm_year >= 100 && tm.tm_year < 1900))) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,

Seems like another abuse of internal error, but I don't know what to use here,
properly.  Maybe VIR_ERR_XML_DETAIL?

> +                               _("Invalid BIOS 'date' format: %s"),
> +                               def->sysinfo->bios_date);

Unnecessarily long, you can do 's/def->sysinfo->bios_//' and save one
line here ;-)

> +                goto error;
> +            }
> +        }
>      }
>  
>      if ((tmp = virXPathString("string(./os/smbios/@mode)", ctxt))) {
> 

I find it a bit harder to read.  Wouldn't this be more nicer if we used
sscanf()?  Or we could take care a bit about the date and do it even
shorter with strptime(), something like this:

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d55ce6b..61f385c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11588,6 +11588,20 @@ virDomainDefParseXML(xmlDocPtr xml,
                 goto error;
             }
         }
+        if (def->sysinfo->bios_date != NULL) {
+            char *date = def->sysinfo->bios_date;
+            char *end;
+            struct tm tm;
+            memset(&tm, 0, sizeof(struct tm));
+
+            end = strptime(date, "%D", &tm);
+
+            if (!end || *end != '\0') {
+                virReportError(VIR_ERR_XML_DETAIL,
+                               _("Invalid BIOS 'date' format: %s"), date);
+                goto error;
+            }
+        }
     }

     if ((tmp = virXPathString("string(./os/smbios/@mode)", ctxt))) {
--

Or should we allow even dates like "99/99/9999"?

Martin

P.S.: I don't mean to be rude with nit-picking, but a test for that
would be nice ;-)

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list