SetLogFileNameA implementation is broken

[renatosilva]

This function is currently broken because it uses agnostic functions like _tcslen and _tcsncpy for dealing with a bare char parameter. This will fail when the function is compiled with _UNICODE defined, for example when some Unicode application links directly to exchndl2.cpp.

Also, I think the A/W suffix is an implementation detail to be automatically handled by the Unicode macros, so I would rename the function to SetLogFileName. The application could either require Unicode or ANSI builds of exchndl DLL with the same function name, or this name could be provided as a macro that resolved to appropriate implementations contained in same DLL.

[jrfonseca]

This function is currently broken because it uses agnostic functions like _tcslen and _tcsncpy for dealing with a bare char parameter.

The code is full of that. Anyway _UNICODE is never defined, so they are equivalent to char.

Yes, it's not nice, but no it's not broken because of that.

Also, I think the A/W suffix is an implementation detail to be automatically handled by the Unicode macros, so I would rename the function to SetLogFileName. The application could either require Unicode or ANSI builds of exchndl DLL with the same function name, or this name could be provided as a macro that resolved to appropriate implementations contained in same DLL.

No, that's not how these things usually work. The DLL provide both A and W entrypoints. Then there is a

#ifdef _UNICODE
#  define SetLogFileName SetLogFileNameW
#else
#  define SetLogFileName SetLogFileNameA
#endif

Anyway, for now I'll be only providing the A entry-point, as I don't use Unicode consistently internally.

[renatosilva]

@jrfonseca, as I said if you compile and link directly against exchndl2.cpp _UNICODE may be defined and you will be passing bare chars to wide functions. It's documented in your readme. Is there any reason you cannot make the function parameter into TCHAR?

Are you planning to provide such a header, possibly together with static and import libraries? It would make usage simpler.

[jrfonseca]

@jrfonseca, as I said if you compile and link directly against exchndl2.cpp _UNICODE may be defined and you will be passing bare chars to wide functions. It's documented in your readme.

I still don't understand what TCHAR you're talking about. SetLogFileNameA prototype has no TCHAR, and neither has sample/exchndl2.cpp, and the README does talk about TCHAR or UNICODE.

Is there any reason you cannot make the function parameter into TCHAR?

Yes, I have no intention of making two versions of exchndl.dll -- one with and without UNICODE. It would be needless overhead. Therefore the solution later on will be two entry points.

For now, only ASCII is supported, hence a caller that wants to use unicode will have to converto ASCII.

Are you planning to provide such a header,

Yes.

possibly together with static and import libraries? It would make usage simpler.

Actually, I think I won't provide import libraries after all -- MinGW's g++ is able to link directly against .DLLs, so one can just do

g++ -o foo.exe foo.cpp /path/to/exchnd.dll

and trust me this is way simpler than dealing with import libraries.

[renatosilva]

Import libraries are more convenient in MSYS2/MinGW-w64 as we can simply pass -lexchndl instead of determining where the DLL is located according to architecture, and we also avoid warnings. This patch from @Alexpux works for me, would you apply?

[jrfonseca]

This patch from @Alexpux works for me, would you apply?

It's totally busted:

• mgwhelp.dll on 32-bits exports now have @xxxx suffix, so mgwhelp no longer works as a drop-in replacement for dbghelp.dll
• ditto for exchndl.dll (so GetProcAddress( " ExcHndlSetLogFileNameA") also fails)

[Alexpux]

Ok, according to http://www.willus.com/mingw/yongweiwu_stdcall.html we need fix DEF files for 32-bit to create aliases for every function like: Function = Function@n Fixed in patch: https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-drmingw-git/import-libs.patch

[jrfonseca]

I'm afraid this is still wrong... Now both Foo@123 and Foo are exported for every function, and god knows which one gets linked in the end.

MinGW support for .DEF is full of bugs. I've tried many times, and I have yet to find a way of producing both .dll and .a that actually handles the __stdcall symbols properly. Either the .dll or .a gets busted. The only solution is to generate the .dll and import lib separately, like was being done in https://github.com/jrfonseca/drmingw/blob/3a57362f2ea8cb84c9551a923775da0008b62fea/src/mgwhelp/CMakeLists.txt#L29-L42

But the easiest way is by far just linking .DLL files directly...

[Alexpux]

What problem with both exported Foo@123 and Foo? You create alias in DEF file Foo = Foo@123 so always called the SAME function.

[jrfonseca]

Why doesn't kernel32.dll provide @123 symbols? Because the standard is for exported symbols on DLLs not to have the @123 suffix.

If at the end of the day, we provide both but the apps always link against the @123, then what's the point of providing the non @123?

Sometimes I wonder if it wouldn't be better just to use __cdecl and avoid all this. But it kinds of annoys me that MinGW is broken to the point it makes so hard to replicate what MSVC linker does. I just want ExcHndl to have the same sort of API like DbgHelp has.

[Alexpux]

We neeed provide non @123 function to get working GetProcAddress function.

[jrfonseca]

Done for ExcHndl in bc67b754dbb68b481689b002a7ce07038df5a36e

PS: @renatosilva, please file new issues for actionable requests. Feel free to refer comments on old issues for context though.