| Message ID | 20170522211205.14265-11-hpoussin@reactos.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <hpoussin@reactos.org> 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 <hpoussin@reactos.org>
> ---
> 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
>
>
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?
On Mon, Aug 7, 2017 at 7:07 AM, Eric Blake <eblake@redhat.com> 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!
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) {
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 <hpoussin@reactos.org> --- block/vvfat.c | 62 ++++++++++++++++++++++++++++------------------------------- 1 file changed, 29 insertions(+), 33 deletions(-)