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

[Pranith Kumar]

On Mon, Aug 7, 2017 at 7:07 AM, Eric Blake  wrote:
> On 08/05/2017 01:52 PM, Pranith Kumar wrote:
>> 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);
>>  ^
>
> Doesn't commit 7c8730d45f6 fix that?

Indeed it does. I hadn't rebased my branch before reporting. Sorry for
the noise!

-- 
Pranith

Re: [Qemu-block] [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  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 
> ---
>  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 < 99; 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 -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