[libvirt] [PATCH] virsysinfo: Use more virSkipSpacesBackwards()

Michal Privoznik posted 1 patch 6 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/46af530542be4867e8899ad2a1bbb10f107c42fe.1520954506.git.mprivozn@redhat.com
Test syntax-check passed
src/util/virsysinfo.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
[libvirt] [PATCH] virsysinfo: Use more virSkipSpacesBackwards()
Posted by Michal Privoznik 6 years, 1 month ago
Some fields reported by dmidecode have plenty of useless spaces
(in fact some have nothing but spaces). To deal with this we have
introduced virSkipSpacesBackwards() and use it in
virSysinfoParseX86Processor() and virSysinfoParseX86Memory().
However, other functions (e.g. virSysinfoParseX86Chassis()) don't
use it at all and thus we are reporting nonsense:

  <sysinfo type='smbios'>
    <chassis>
      <entry name='manufacturer'>FUJITSU</entry>
      <entry name='version'>                      </entry>
      <entry name='serial'>                </entry>
      <entry name='asset'>                                        </entry>
      <entry name='sku'>Default string</entry>
    </chassis>
  </sysinfo>

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virsysinfo.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index 43a4c8835..5795d90c7 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -644,7 +644,8 @@ static int
 virSysinfoParseBIOS(const char *base, virSysinfoBIOSDefPtr *bios)
 {
     int ret = -1;
-    const char *cur, *eol = NULL;
+    const char *cur;
+    char *eol = NULL;
     virSysinfoBIOSDefPtr def;
 
     if ((cur = strstr(base, "BIOS Information")) == NULL)
@@ -657,24 +658,28 @@ virSysinfoParseBIOS(const char *base, virSysinfoBIOSDefPtr *bios)
     if ((cur = strstr(base, "Vendor: ")) != NULL) {
         cur += 8;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->vendor, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "Version: ")) != NULL) {
         cur += 9;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "Release Date: ")) != NULL) {
         cur += 14;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->date, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "BIOS Revision: ")) != NULL) {
         cur += 15;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->release, cur, eol - cur) < 0)
             goto cleanup;
     }
@@ -697,7 +702,8 @@ static int
 virSysinfoParseX86System(const char *base, virSysinfoSystemDefPtr *sysdef)
 {
     int ret = -1;
-    const char *cur, *eol = NULL;
+    const char *cur;
+    char *eol = NULL;
     virSysinfoSystemDefPtr def;
 
     if ((cur = strstr(base, "System Information")) == NULL)
@@ -710,42 +716,49 @@ virSysinfoParseX86System(const char *base, virSysinfoSystemDefPtr *sysdef)
     if ((cur = strstr(base, "Manufacturer: ")) != NULL) {
         cur += 14;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->manufacturer, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "Product Name: ")) != NULL) {
         cur += 14;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->product, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "Version: ")) != NULL) {
         cur += 9;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "Serial Number: ")) != NULL) {
         cur += 15;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "UUID: ")) != NULL) {
         cur += 6;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->uuid, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "SKU Number: ")) != NULL) {
         cur += 12;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->sku, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "Family: ")) != NULL) {
         cur += 8;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->family, cur, eol - cur) < 0)
             goto cleanup;
     }
@@ -770,7 +783,8 @@ virSysinfoParseX86BaseBoard(const char *base,
                             size_t *nbaseBoard)
 {
     int ret = -1;
-    const char *cur, *eol = NULL;
+    const char *cur;
+    char *eol = NULL;
     virSysinfoBaseBoardDefPtr boards = NULL;
     size_t nboards = 0;
     char *board_type = NULL;
@@ -787,36 +801,42 @@ virSysinfoParseX86BaseBoard(const char *base,
         if ((cur = strstr(base, "Manufacturer: ")) != NULL) {
             cur += 14;
             eol = strchr(cur, '\n');
+            virSkipSpacesBackwards(cur, &eol);
             if (eol && VIR_STRNDUP(def->manufacturer, cur, eol - cur) < 0)
                 goto cleanup;
         }
         if ((cur = strstr(base, "Product Name: ")) != NULL) {
             cur += 14;
             eol = strchr(cur, '\n');
+            virSkipSpacesBackwards(cur, &eol);
             if (eol && VIR_STRNDUP(def->product, cur, eol - cur) < 0)
                 goto cleanup;
         }
         if ((cur = strstr(base, "Version: ")) != NULL) {
             cur += 9;
             eol = strchr(cur, '\n');
+            virSkipSpacesBackwards(cur, &eol);
             if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0)
                 goto cleanup;
         }
         if ((cur = strstr(base, "Serial Number: ")) != NULL) {
             cur += 15;
             eol = strchr(cur, '\n');
+            virSkipSpacesBackwards(cur, &eol);
             if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0)
                 goto cleanup;
         }
         if ((cur = strstr(base, "Asset Tag: ")) != NULL) {
             cur += 11;
             eol = strchr(cur, '\n');
+            virSkipSpacesBackwards(cur, &eol);
             if (eol && VIR_STRNDUP(def->asset, cur, eol - cur) < 0)
                 goto cleanup;
         }
         if ((cur = strstr(base, "Location In Chassis: ")) != NULL) {
             cur += 21;
             eol = strchr(cur, '\n');
+            virSkipSpacesBackwards(cur, &eol);
             if (eol && VIR_STRNDUP(def->location, cur, eol - cur) < 0)
                 goto cleanup;
         }
@@ -848,7 +868,8 @@ virSysinfoParseX86Chassis(const char *base,
                           virSysinfoChassisDefPtr *chassisdef)
 {
     int ret = -1;
-    const char *cur, *eol = NULL;
+    const char *cur;
+    char *eol = NULL;
     virSysinfoChassisDefPtr def;
 
     if ((cur = strstr(base, "Chassis Information")) == NULL)
@@ -861,30 +882,35 @@ virSysinfoParseX86Chassis(const char *base,
     if ((cur = strstr(base, "Manufacturer: ")) != NULL) {
         cur += 14;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->manufacturer, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "Version: ")) != NULL) {
         cur += 9;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "Serial Number: ")) != NULL) {
         cur += 15;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "Asset Tag: ")) != NULL) {
         cur += 11;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->asset, cur, eol - cur) < 0)
             goto cleanup;
     }
     if ((cur = strstr(base, "SKU Number: ")) != NULL) {
         cur += 12;
         eol = strchr(cur, '\n');
+        virSkipSpacesBackwards(cur, &eol);
         if (eol && VIR_STRNDUP(def->sku, cur, eol - cur) < 0)
             goto cleanup;
     }
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsysinfo: Use more virSkipSpacesBackwards()
Posted by Andrea Bolognani 6 years, 1 month ago
On Tue, 2018-03-13 at 16:21 +0100, Michal Privoznik wrote:
> Some fields reported by dmidecode have plenty of useless spaces
> (in fact some have nothing but spaces). To deal with this we have
> introduced virSkipSpacesBackwards() and use it in
> virSysinfoParseX86Processor() and virSysinfoParseX86Memory().
> However, other functions (e.g. virSysinfoParseX86Chassis()) don't
> use it at all and thus we are reporting nonsense:
> 
>   <sysinfo type='smbios'>
>     <chassis>
>       <entry name='manufacturer'>FUJITSU</entry>
>       <entry name='version'>                      </entry>
>       <entry name='serial'>                </entry>
>       <entry name='asset'>                                        </entry>
>       <entry name='sku'>Default string</entry>
>     </chassis>
>   </sysinfo>
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/virsysinfo.c | 34 ++++++++++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)

This would be the perfect time to add test coverage for such odd
smbios data, wouldn't it? :)

You can just modify tests/sysinfodata/x86sysinfo.data and add some
extra spaces eg. after "LENOVO" to show that they are now ignored.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsysinfo: Use more virSkipSpacesBackwards()
Posted by Michal Privoznik 6 years, 1 month ago
On 03/14/2018 10:15 AM, Andrea Bolognani wrote:
> On Tue, 2018-03-13 at 16:21 +0100, Michal Privoznik wrote:
>> Some fields reported by dmidecode have plenty of useless spaces
>> (in fact some have nothing but spaces). To deal with this we have
>> introduced virSkipSpacesBackwards() and use it in
>> virSysinfoParseX86Processor() and virSysinfoParseX86Memory().
>> However, other functions (e.g. virSysinfoParseX86Chassis()) don't
>> use it at all and thus we are reporting nonsense:
>>
>>   <sysinfo type='smbios'>
>>     <chassis>
>>       <entry name='manufacturer'>FUJITSU</entry>
>>       <entry name='version'>                      </entry>
>>       <entry name='serial'>                </entry>
>>       <entry name='asset'>                                        </entry>
>>       <entry name='sku'>Default string</entry>
>>     </chassis>
>>   </sysinfo>
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/util/virsysinfo.c | 34 ++++++++++++++++++++++++++++++----
>>  1 file changed, 30 insertions(+), 4 deletions(-)
> 
> This would be the perfect time to add test coverage for such odd
> smbios data, wouldn't it? :)
> 
> You can just modify tests/sysinfodata/x86sysinfo.data and add some
> extra spaces eg. after "LENOVO" to show that they are now ignored.
> 

Okay, added and pushed. Thanks.

Michal

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