[Qemu-devel] [PATCH] block/vvfat: Fix compiler warnings for OpenBSD

[Qemu-devel] [PATCH] block/vvfat: Fix compiler warnings for OpenBSD

[Stefan Weil]

The buildbot shows these compiler warnings:

block/vvfat.c: In function 'create_short_and_long_name':
block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
block/vvfat.c:635: warning: array size (8) smaller than bound length (11)
block/vvfat.c:635: warning: array size (8) smaller than bound length (11)

They are caused by tricky code where 8 characters for the name are followed
by 3 characters for the extension, and some operations touch both name and
extension.

Using an 11 character name which includes the extension fixes the compiler
warning, satisfies cppcheck, valgrind and maybe other static and dynamic
code checkers, and even simplifies some parts of the code.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 block/vvfat.c |   43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 3ddaa0b..1abb8ad 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -266,8 +266,7 @@ typedef struct mbr_t {
 } QEMU_PACKED mbr_t;
 
 typedef struct direntry_t {
-    uint8_t name[8];
-    uint8_t extension[3];
+    uint8_t name[8 + 3];
     uint8_t attributes;
     uint8_t reserved[2];
     uint16_t ctime;
@@ -518,11 +517,9 @@ static inline uint8_t fat_chksum(const direntry_t* entry)
     uint8_t chksum=0;
     int i;
 
-    for(i=0;i<11;i++) {
-        unsigned char c;
-
-        c = (i < 8) ? entry->name[i] : entry->extension[i-8];
-        chksum=(((chksum&0xfe)>>1)|((chksum&0x01)?0x80:0)) + c;
+    for (i = 0; i < ARRAY_SIZE(entry->name); i++) {
+        chksum = (((chksum & 0xfe) >> 1) |
+                  ((chksum & 0x01) ? 0x80 : 0)) + entry->name[i];
     }
 
     return chksum;
@@ -617,7 +614,7 @@ static inline direntry_t* create_short_and_long_name(BDRVVVFATState* s,
 
     if(is_dot) {
 	entry=array_get_next(&(s->directory));
-	memset(entry->name,0x20,11);
+        memset(entry->name, 0x20, sizeof(entry->name));
 	memcpy(entry->name,filename,strlen(filename));
 	return entry;
     }
@@ -632,12 +629,14 @@ static inline direntry_t* create_short_and_long_name(BDRVVVFATState* s,
 	i = 8;
 
     entry=array_get_next(&(s->directory));
-    memset(entry->name,0x20,11);
+    memset(entry->name, 0x20, sizeof(entry->name));
     memcpy(entry->name, filename, i);
 
-    if(j > 0)
-	for (i = 0; i < 3 && filename[j+1+i]; i++)
-	    entry->extension[i] = filename[j+1+i];
+    if (j > 0) {
+        for (i = 0; i < 3 && filename[j + 1 + i]; i++) {
+            entry->name[8 + i] = filename[j + 1 + i];
+        }
+    }
 
     /* upcase & remove unwanted characters */
     for(i=10;i>=0;i--) {
@@ -861,8 +860,7 @@ static int init_directories(BDRVVVFATState* s,
     {
 	direntry_t* entry=array_get_next(&(s->directory));
 	entry->attributes=0x28; /* archive | volume label */
-	memcpy(entry->name,"QEMU VVF",8);
-	memcpy(entry->extension,"AT ",3);
+        memcpy(entry->name, "QEMU VVFAT ", sizeof(entry->name));
     }
 
     /* Now build FAT, and write back information into directory */
@@ -1591,17 +1589,20 @@ static int parse_short_name(BDRVVVFATState* s,
 	    lfn->name[i] = direntry->name[i];
     }
 
-    for (j = 2; j >= 0 && direntry->extension[j] == ' '; j--);
+    for (j = 2; j >= 0 && direntry->name[8 + j] == ' '; j--) {
+    }
     if (j >= 0) {
 	lfn->name[i++] = '.';
 	lfn->name[i + j + 1] = '\0';
 	for (;j >= 0; j--) {
-	    if (direntry->extension[j] <= ' ' || direntry->extension[j] > 0x7f)
-		return -2;
-	    else if (s->downcase_short_names)
-		lfn->name[i + j] = qemu_tolower(direntry->extension[j]);
-	    else
-		lfn->name[i + j] = direntry->extension[j];
+            uint8_t c = direntry->name[8 + j];
+            if (c <= ' ' || c > 0x7f) {
+                return -2;
+            } else if (s->downcase_short_names) {
+                lfn->name[i + j] = qemu_tolower(c);
+            } else {
+                lfn->name[i + j] = c;
+            }
 	}
     } else
 	lfn->name[i + j + 1] = '\0';
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] block/vvfat: Fix compiler warnings for OpenBSD

[Stefan Hajnoczi]

On Wed, Dec 11, 2013 at 09:37:11PM +0100, Stefan Weil wrote:
> The buildbot shows these compiler warnings:
> 
> block/vvfat.c: In function 'create_short_and_long_name':
> block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
> block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
> block/vvfat.c:635: warning: array size (8) smaller than bound length (11)
> block/vvfat.c:635: warning: array size (8) smaller than bound length (11)
> 
> They are caused by tricky code where 8 characters for the name are followed
> by 3 characters for the extension, and some operations touch both name and
> extension.
> 
> Using an 11 character name which includes the extension fixes the compiler
> warning, satisfies cppcheck, valgrind and maybe other static and dynamic
> code checkers, and even simplifies some parts of the code.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  block/vvfat.c |   43 ++++++++++++++++++++++---------------------
>  1 file changed, 22 insertions(+), 21 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH] block/vvfat: Fix compiler warnings for OpenBSD

[Kevin Wolf]

Am 12.12.2013 um 09:22 hat Stefan Hajnoczi geschrieben:
> On Wed, Dec 11, 2013 at 09:37:11PM +0100, Stefan Weil wrote:
> > The buildbot shows these compiler warnings:
> > 
> > block/vvfat.c: In function 'create_short_and_long_name':
> > block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
> > block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
> > block/vvfat.c:635: warning: array size (8) smaller than bound length (11)
> > block/vvfat.c:635: warning: array size (8) smaller than bound length (11)
> > 
> > They are caused by tricky code where 8 characters for the name are followed
> > by 3 characters for the extension, and some operations touch both name and
> > extension.
> > 
> > Using an 11 character name which includes the extension fixes the compiler
> > warning, satisfies cppcheck, valgrind and maybe other static and dynamic
> > code checkers, and even simplifies some parts of the code.
> > 
> > Signed-off-by: Stefan Weil <sw@weilnetz.de>
> > ---
> >  block/vvfat.c |   43 ++++++++++++++++++++++---------------------
> >  1 file changed, 22 insertions(+), 21 deletions(-)
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH] block/vvfat: Fix compiler warnings for OpenBSD

[Brad Smith]

On 11/12/13 3:37 PM, Stefan Weil wrote:
> The buildbot shows these compiler warnings:
>
> block/vvfat.c: In function 'create_short_and_long_name':
> block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
> block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
> block/vvfat.c:635: warning: array size (8) smaller than bound length (11)
> block/vvfat.c:635: warning: array size (8) smaller than bound length (11)

This is due to an OpenBSD specific GCC bounds checker.

      -   gcc recognizes a new flag, -Wbounded, to perform basic checks
          on functions which accept buffers and sizes.  An extra
          attribute, __bounded__, has been added to mark functions that
          can be checked this way.

There was another issue here..

   CC    hw/display/exynos4210_fimd.o
hw/display/exynos4210_fimd.c: In function 'exynos4210_fimd_reset':
hw/display/exynos4210_fimd.c:1342: warning: array size (16) smaller than 
bound length (192)
hw/display/exynos4210_fimd.c:1342: warning: array size (16) smaller than 
bound length (192)

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.