Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor build error if compiled by WinXP-compatible MSVC v141_xp SDK #86

Open
NikolajSchlej opened this issue Feb 14, 2025 · 4 comments
Open

Comments

@NikolajSchlej
Copy link

Hi @Dax89,

I've updated my local copy of QHexView to the current state, and it went mostly smoothly if not for one minor build issue when built with an obscure configuration.

As I'm still maintaining Windows XP compatibility for Win32 versions of UEFITool, UEFIExtract and UEFIFind, all libraries that are in use are built with an ancient v141_xp SDK that is still supported by MS in various Visual Studio releases. I will continue supporting it until GitHub stops providing a VM to build such binaries with, which will likely happen next year.

Way back then, tolower() had not been a member of std for MSVC, and I had to fall back to use the C version from ctype.h to make qhexutils.cpp compile again. There are only 2 instances to replace:

                if(!(options & QHexFindOptions::CaseSensitive)) {
                    ch1 = std::tolower(ch1);
                    ch2 = std::tolower(ch2);
                }

Thank you once again for this library, always a pleasure to use.

  • Nikolaj
@Dax89
Copy link
Owner

Dax89 commented Feb 14, 2025

Hi!
Can you add and test this code in src/model/qhexutils.cpp?

#if defined(_WIN32) && _MSC_VER <= 1916 // v141_xp
#include <ctype.h>
namespace std {
using ::tolower;
}
#else
#include <cctype>
#endif

I have done some research: it seems that _MSC_VER for that version of VC++ is 1916, but somewhere it says 1900, so I'm not 100% sure that this code works.

MSDN Link: https://learn.microsoft.com/en-us/cpp/overview/compiler-versions?view=msvc-170

@NikolajSchlej
Copy link
Author

I will test that, sure, but it's unclear to me why not directly use the C function that is compatible with everything, instead of adding more conditional compilation with somewhat unclear conditions.

My patch was just

#include <ctype.h>
...
                if(!(options & QHexFindOptions::CaseSensitive)) {
                    ch1 = tolower(ch1);
                    ch2 = tolower(ch2);
                }

which is already proven to work everywhere.

Are there some locale/Unicode interaction that C++ function covers but older C function doesn't?

@Dax89
Copy link
Owner

Dax89 commented Feb 15, 2025

It's just a personal policy: in C++ codebases I prefer to use C++ includes (if possible) even if they are just wrapper/alias around C functions, in general I prefer to reduce C/C++ code mixing at minimum.

If the conditional compilation doesn't work I will use ctype.h as you suggested!

@NikolajSchlej
Copy link
Author

Tried your patch variant, it works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants