Fix three bugs found in issue 20 #22
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The output of DateTimeStrg() was missing a terminating null. This causes crashes when one tries to includes it later as part of "/* VTT %S compiler %S */".
…licit casting to long This is a variation of the issue with 32-bit long on Windows. On 64-bit Linux, these %li causes casting to 64-bit types. A negative number becomes a large positive value, 4294967296 + number, and generates "Number too large to be parsed, larger than 32,767" errors, when used with the "-a compile everything" option. %li in other files than TTEngine.cpp applies to user messages, instead, and for now, left unchanged. Part of microsoft#20
…pecific macros later.
Done with:
sed -i -e 's/L"%s"/WIDE_STR_FORMAT/' *.cpp
sed -i -e 's/%s"/" WIDE_STR_FORMAT/g' *.cpp
sed -i -e 's/L"%s/WIDE_STR_FORMAT L"/g' *.cpp
sed -i -e 's/%s/" WIDE_STR_FORMAT L"/g' *.cpp
Except one NARROW_STR_FORMAT.
Conflicts:
src/TTFont.cpp
src/vttcompile.cpp
… used Reference: https://docs.microsoft.com/en-us/cpp/c-runtime-library/format-specification-syntax-printf-and-wprintf-functions?view=msvc-160 " The arguments that follow the format string are interpreted according to the corresponding type character and the optional size prefix. Conversions for character types char and wchar_t are specified by using c or C, and single-byte and multi-byte or wide character strings are specified by using s or S, depending on which formatting function is being used. Character and string arguments that are specified by using c and s are interpreted as char and char* by printf family functions, or as wchar_t and wchar_t* by wprintf family functions. Character and string arguments that are specified by using C and S are interpreted as wchar_t and wchar_t* by printf family functions, or as char and char* by wprintf family functions. This behavior is Microsoft-specific. Microsoft-specific: The Z type character, and the behavior of the c, C, s, and S type characters when they're used with the printf and wprintf functions, are Microsoft extensions. The ISO C standard uses c and s consistently for narrow characters and strings, and C and S for wide characters and strings, in all formatting functions. " This is the truncated string problem seen in microsoft#20
Done with:
sed -i -e 's/%hs/" NARROW_STR_FORMAT L"/g' *.cpp
Found by:
grep -n %li *.cpp | grep -v ErrMsg, | grep -v errMsg, | grep -v error,
|
There seems to be only two %li which are not error messages. They don't involve negative values so unlikely to be important; so that is included for completeness (seeing as the other changes in TTEngine.cpp are important). |
|
This is all for #20 - I hope this makes non-windows build compatible with windows build now, but would be nice to have test files as well as test cases. |
|
This looks great, thanks! |
|
Yeah, with this bunch of changes, vttcompile more or less works correctly on 64-bit linux. I just filed an issue of the difference between vttcompile on 64-bit linux and 32-bit vttshell on wine - #23 . Maybe you can see the difference with vttcompile on windows vs vttshell on windows too. BTW, you should be able to build and run 64-bit vttcompile for linux under WSL. That should enable you to look at difference between vttcompile on linux vs vttshell. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The test case in issue 20 uncovers 3 problems:
Microsoft compiler treats %s passed to wprintf(), different from everybody else. So we need to special-case Microsoft compiler with one wide-string-format specifier, and everybody else with a different one. (and vice versa for narrow string - but there is only one case, in main()).
After fixing the truncation, I found a missing null termination problem with the previous work-around for unix systems lacking a wctime() function (and need to call ctime() then convert from narrow to wide).
and a similar problem with earlier long to int32_t, needs to replace a lot of %li by %i to not implicit convert to 64-bit long.
I only replaced %li in TTEngine.cpp ; the other seems to affect user messages (if error) and not critical.