Menu

#2326 Use _snprintf in mingwex/dirent.c

WSL
closed
None
Feature
fixed
IINR_-_Include_In_Next_Release
True
2017-01-26
2017-01-05
Jason Hood
No

This is a patch to use _sn(w)printf for mingwex/dirent.c DIRENT_ASSIGN_NAME,
to avoid including the enhanced ISO versions unnecessarily (there's no issue
with the potential lack of null, as the count is 260 and the filesystem limit
for a name is 255). This allows small programs (using globbing) to stay small.
Size comparison:

T:\>echo int main(void) { return 0; }>test.c
T:\>gcc test.c -s -o old.exe
T:\>gcc test.c -s -o new.exe -Lmingwrt-3.22.4-1
T:\>ls -l old.exe new.exe
-rwxrwxrwx  1 admin 0 18432 2017-01-05 09:41 new.exe
-rwxrwxrwx  1 admin 0 46080 2017-01-05 09:41 old.exe
1 Attachments

Discussion

  • Keith Marshall

    Keith Marshall - 2017-01-08

    Thanks for the patch. However, I am extremely reluctant to introduce a dependency on any Microsoft API which is as badly broken as their _snprintf() function; even if the probability that the broken aspect would be triggered is low, it is not zero.

    I agree that the overhead of invoking the MinGW snprintf() may be perceived as significant, in the context of trivial test cases; perhaps less significant for real-world applications, (and it vanishes altogether, for applications which rely on ISO-C99 printf() capability). That said, we could significantly reduce the permanent overhead, by adopting the attached alternative patch, which would have the additional benefit of eliminating an existing dependency on Microsoft's equally badly broken _snwprintf() function, (to which MinGW's snwprintf() is redirected), in the UTF-16LE variant of the directory stream handling functions.

     
  • Jason Hood

    Jason Hood - 2017-01-09

    I know it's not much of a real-world issue, but it still bothers me, so thanks for not dismissing it (it's bad enough that a small MinGW program used to be 12k; 18k I can live with, but 46 is just too much).

     
  • Keith Marshall

    Keith Marshall - 2017-01-26
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -1,12 +1,13 @@
    -This is a patch to use _sn(w)printf for mingwex/dirent.c DIRENT_ASSIGN_NAME,
    +This is a patch to use `_sn(w)printf` for mingwex/dirent.c `DIRENT_ASSIGN_NAME`,
     to avoid including the enhanced ISO versions unnecessarily (there's no issue
     with the potential lack of null, as the count is 260 and the filesystem limit
     for a name is 255).  This allows small programs (using globbing) to stay small.
     Size comparison:
    -
    +~~~~
     T:\>echo int main(void) { return 0; }>test.c
     T:\>gcc test.c -s -o old.exe
     T:\>gcc test.c -s -o new.exe -Lmingwrt-3.22.4-1
     T:\>ls -l old.exe new.exe
     -rwxrwxrwx  1 admin 0 18432 2017-01-05 09:41 new.exe
     -rwxrwxrwx  1 admin 0 46080 2017-01-05 09:41 old.exe
    +~~~~
    
    • status: unread --> closed
    • assigned_to: Keith Marshall
    • Resolution: none --> fixed
    • Category: Unknown --> IINR_-_Include_In_Next_Release
     
  • Keith Marshall

    Keith Marshall - 2017-01-26

    I committed [df96ca], as my preferred alternative implementation.

     

    Related

    Commit: [df96ca]