On Wednesday 17 June 2026 06:39:01 Kirill Makurin wrote: > Pali Rohár <[email protected]> wrote: > > > Hello, I looked at changes and I have few comments for some of them. See > > below. > > > > ... > > > > For these two macros there is missing opposite direction of xfail. > > If the test passes and xfail is set then it should return 1 (indicate > > failure). > > I'm not sure about it. In the current state, this test will pass regardless > of whether `_aligned_realloc` is buggy or not; if any subtest fails, there > will be output in log files. > > If we were to make the test fail when an `xfail` subtest passes, we would hit > a random CI failure at some point and would need to fix this test. Doing so, > would make this test fail on all older systems without the fix. > > What do others think?
Point of xfail is not to skip the test but mark it that the result of it is opposed. > > Those ifdefs for library name is really ugly. I needed it more times and > > I was thinking about this idea: > > > > What about providing one specific symbol into every mingw-w64 CRT import > > library which would be a global char array with content of the library > > name? > > > > With this, it would be needed just to reference that symbol (e.g. > > __mingw_crt_library_name) and could be used in any application which > > needs this. > > I agree that this is pretty ugly, but it was the best way to do it locally. I > actually like your idea, except I would prefer to keep this stuff for > internal use only. > > In posix32, I have function `p32_crt_handle`[1] which returns handle to CRT > the library is linked against (or terminates the process if it failed); when > linked against static CRT (MSVC with /MT switches), it returns zero to > inidcate static CRT. We might as well do something similar. > > [1] > https://github.com/maiddaisuki/posix32/blob/master/lib/posix32-core/src/crt.c > > > Another idea: Via WinAPI it is possible to get HMODULE from any passed > > address of function. So what about using address of exit function (which > > is available in every CRT library) then deduce HMODULE of it (which > > would be the currently linked CRT DLL library) and on that call > > GetProcAddress? > > I really forgot that it was an option. I think your idea above is generally > better. I think that this is the best solution for runtime. But in my opinion this case is the "linking" problem because at the link time the linker knows to which library it is linking, so it could provide this information to application via link time symbol. And that was my idea above adding new "char __mingw_crt_library_name[]" symbol into every CRT import library. > > I think that this test should still asserts that the lpCmdLine[0] is not > > '\0' > > like it was done before. > > This was the initial idea, but then assertion has failed; when you pass no > args to executable which uses `WinMain` as entry point, `lpCmdLine` will be > an empty string. Ou, right the lpCmdLine does not contain argv[0] or application name. I forgot about it. > > The proper way to compile windows gui application is to pass -mwindows > > command line switch. It will properly sets subsystem to gui and > > correctly choose required gui library. So the -lgdi32 is not needed in > > this case. > > I really forgot that -mwindows exists. Nice catch, fix is in attachment. > > > This is not quite correct. Usage of __MINGW_USE_ANSI_STDIO replaces > > _all_ usage of (ms)printf by (mingw)printf. I think that the purpose of > > test was to check that code works correctly with the default (possible) > > (ms)printf functions. And just usage of long double needs to be done > > explicitly by (mingw)printf as the ms variant has no 80-bit float > > support. > > > > So IMHO the previous code was correct. > > I think these tests have nothing to do with testing printf's formatting > capabilities and the output is just for debugging. I assume that back when > this test was written, you would simply compile and run the tests, and > examine output to see if anything is odd. > > If we want to test printf's formatting capabilities with floating point > numbers, I think we need a separate test for it. Ok. Quite hard to determinate the original purpose of the test if it does not have the description. > > nit: There is a mix of TABs and spaces (something which you tried to > > clean in previous changes in series). > > Nice catch; fix is in attachment. > > > assert here is wrong. If the _set_error_mode fails then the assert will > > pop-up message box. > > Hm... I copied this block from my posix32 library, where I ran this code > hundreds if not thousands of times with almost every CRT. I have never hit an > error return from `_set_error_mode`. Well, correctly written test for correct code never hits any assert :-) The purpose is to catch unexpected issues. > What others think? > > > I would propose to call this function at the beginning of main() before > > declaring or initializing any variable, to ensure that it is always > > called at the beginning. Initialization of variable could be done by > > function call which could potentially cause assert/failure. > > I'm certain that I made sure that in every test which calls > `mingw_test_init`, this it is the first function called. Ok. If it was correctly checked/reviewed I'm not going to oppose it. But it does not have to be so obvious for new developer who is going to read the code. And lot of people are writing new tests by copying some existing and modifying it (old test is used as a template). And this can cause an issue because if modifying existing code, people can forgot about the fact that adding a new variable with initialization from function is forbidden for main. So I think it is safer to put the mingw_test_init immediately after the main, that would be explicit for everybody that variables needs to be initialized after that call. Visually it looks like this: int main() { mingw_test_init(); int my_variable = some_function(); int ret = 0; ... return ret; } > I wasn't sure whether I should call `mingw_test_init` before declaration > block. My guess was that the declaration block like this is used or allow > code to compile in strict C89 mode, which does not support mixing > declarations and code. So, I tried to keep it whenever possible. mingw-w64 already requires GNU extensions and calling functions before declaration block is in this case allowed. But you are right that in strict non-GNU C89 it is forbidden. Makefile.am already contains "AM_CFLAGS=-pipe -std=gnu99" so the code is always compiled in GNU C99 mode and it is requirement from compiler. IMHO for me the above code where the mingw_test_init() is called as the first thing immediately under "int main() {" line is more readable. > > - Kirill Makurin _______________________________________________ Mingw-w64-public mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
