Re: [Qemu-devel] [PATCH v2 10/13] vvfat: correctly generate numeric-tail of short file names

[Pranith Kumar]

FYI,

This commit breaks the build with gcc-7:

  CC      block/vvfat.o
qemu/block/vvfat.c: In function ‘read_directory’:
qemu/block/vvfat.c:605:37: error: ‘__builtin___sprintf_chk’ may write
a terminating nul past the end of the destination
[-Werror=format-overflow=]
             int len = sprintf(tail, "~%d", i);
                                     ^~~~~
In file included from /usr/include/stdio.h:938:0,
                 from qemu/include/qemu/osdep.h:68,
                 from qemu/block/vvfat.c:25:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:33:10: note:
‘__builtin___sprintf_chk’ output between 3 and 12 bytes into a
destination of size 11
   return __builtin___sprintf_chk (__s, __USE_FORTIFY_LEVEL - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       __bos (__s), __fmt, __va_arg_pack ());
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
qemu/rules.mak:66: recipe for target 'block/vvfat.o' failed
make: *** [block/vvfat.o] Error 1


Thanks,

On Mon, May 22, 2017 at 5:12 PM, Hervé Poussineau <address@hidden> wrote:
> More specifically:
> - try without numeric-tail only if LFN didn't have invalid short chars
> - start at ~1 (instead of ~0)
> - handle case if numeric tail is more than one char (ie > 10)
>
> Windows 9x Scandisk doesn't see anymore mismatches between short file names 
> and
> long file names for non-ASCII filenames.
>
> Specification: "FAT: General overview of on-disk format" v1.03, page 31
> Signed-off-by: Hervé Poussineau <address@hidden>
> ---
>  block/vvfat.c | 62 
> ++++++++++++++++++++++++++++-------------------------------
>  1 file changed, 29 insertions(+), 33 deletions(-)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 3cb65493cd..414bca6dee 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -529,7 +529,8 @@ static uint8_t to_valid_short_char(gunichar c)
>  }
>
>  static direntry_t *create_short_filename(BDRVVVFATState *s,
> -                                         const char *filename)
> +                                         const char *filename,
> +                                         unsigned int directory_start)
>  {
>      int i, j = 0;
>      direntry_t *entry = array_get_next(&(s->directory));
> @@ -587,8 +588,32 @@ static direntry_t *create_short_filename(BDRVVVFATState 
> *s,
>              }
>          }
>      }
> -    (void)lossy_conversion;
> -    return entry;
> +
> +    /* numeric-tail generation */
> +    for (j = 0; j < 8; j++) {
> +        if (entry->name[j] == ' ') {
> +            break;
> +        }
> +    }
> +    for (i = lossy_conversion ? 1 : 0; i < 999999; i++) {
> +        direntry_t *entry1;
> +        if (i > 0) {
> +            int len = sprintf(tail, "~%d", i);
> +            memcpy(entry->name + MIN(j, 8 - len), tail, len);
> +        }
> +        for (entry1 = array_get(&(s->directory), directory_start);
> +             entry1 < entry; entry1++) {
> +            if (!is_long_name(entry1) &&
> +                !memcmp(entry1->name, entry->name, 11)) {
> +                break; /* found dupe */
> +            }
> +        }
> +        if (entry1 == entry) {
> +            /* no dupe found */
> +            return entry;
> +        }
> +    }
> +    return NULL;
>  }
>
>  /* fat functions */
> @@ -701,36 +726,7 @@ static inline direntry_t* 
> create_short_and_long_name(BDRVVVFATState* s,
>      }
>
>      entry_long=create_long_filename(s,filename);
> -    entry = create_short_filename(s, filename);
> -
> -    /* mangle duplicates */
> -    while(1) {
> -        direntry_t* entry1=array_get(&(s->directory),directory_start);
> -        int j;
> -
> -        for(;entry1<entry;entry1++)
> -            if(!is_long_name(entry1) && !memcmp(entry1->name,entry->name,11))
> -                break; /* found dupe */
> -        if(entry1==entry) /* no dupe found */
> -            break;
> -
> -        /* use all 8 characters of name */
> -        if(entry->name[7]==' ') {
> -            int j;
> -            for(j=6;j>0 && entry->name[j]==' ';j--)
> -                entry->name[j]='~';
> -        }
> -
> -        /* increment number */
> -        for(j=7;j>0 && entry->name[j]=='9';j--)
> -            entry->name[j]='0';
> -        if(j>0) {
> -            if(entry->name[j]<'0' || entry->name[j]>'9')
> -                entry->name[j]='0';
> -            else
> -                entry->name[j]++;
> -        }
> -    }
> +    entry = create_short_filename(s, filename, directory_start);
>
>      /* calculate checksum; propagate to long name */
>      if(entry_long) {
> --
> 2.11.0
>
>

-- 
Pranith