rikyoz / bit7z

A C++ static library offering a clean and simple interface to the 7-zip shared libraries.
https://rikyoz.github.io/bit7z
Mozilla Public License 2.0
611 stars 112 forks source link

[Bug]: Some mistakes about macros #125

Closed nynauy closed 9 months ago

nynauy commented 1 year ago

bit7z version

4.0.x RC

Compilation options

No response

7-zip version

v22.01

7-zip shared library used

7z.dll / 7z.so

Compilers

MSVC

Compiler versions

MSVC 2022

Architecture

x86_64

Operating system

Windows

Operating system versions

No response

Bug description

  1. Attempting to enable BIT7Z_USE_NATIVE_STRING by uncommenting line 17 in bitdefines.hpp will result in a compilation failure as follows:

.../bit7zlibrary.cpp(33): error C2664: "HMODULE LoadLibraryW(LPCWSTR)": Cannot convert argument 1 from "const _Elem *" to "LPCWSTR"

It‘s quite easy to solve the problem. I just include bitdefines.hpp at the beginning of bittypes.hpp.

  1. Maybe line 20 in bit7zlibrary.cpp:

# define Bit7zLoadLibrary(lib_name) LoadLibraryW( WIDEN( (library_path) ).c_str() )

should be

# define Bit7zLoadLibrary(lib_name) LoadLibraryW( WIDEN( (lib_name) ).c_str() )

, corresponding to line 25:

# define Bit7zLoadLibrary( lib_name ) dlopen( (lib_name).c_str(), RTLD_LAZY )

Though it will not cause any errors temporarily.

Steps to reproduce

No response

Expected behavior

No response

Relevant compilation output

No response

Code of Conduct

rikyoz commented 1 year ago

Hi!

  1. Attempting to enable BIT7Z_USE_NATIVE_STRING by uncommenting line 17 in bitdefines.hpp will result in a compilation failure as follows:

.../bit7zlibrary.cpp(33): error C2664: "HMODULE LoadLibraryW(LPCWSTR)": Cannot convert argument 1 from "const _Elem *" to "LPCWSTR"

It‘s quite easy to solve the problem. I just include bitdefines.hpp at the beginning of bittypes.hpp.

Nice catch! I fixed it on the develop branch, so the fix will be available on the final release of the library!

2. Maybe line 20 in bit7zlibrary.cpp:

# define Bit7zLoadLibrary(lib_name) LoadLibraryW( WIDEN( (library_path) ).c_str() )

should be

# define Bit7zLoadLibrary(lib_name) LoadLibraryW( WIDEN( (lib_name) ).c_str() )

, corresponding to line 25:

# define Bit7zLoadLibrary( lib_name ) dlopen( (lib_name).c_str(), RTLD_LAZY )

Though it will not cause any errors temporarily.

Yeah, it's probably an error due to my IDE's automatic refactoring. Fortunately, when the macro is used in Bit7zLibrary's constructor, the argument of this latter is named library_path too, so it shouldn't cause any error (and hence why I never noticed the typo 😅 in my tests). I fixed this too on the develop branch.

Thank you for reporting these mistakes! 🙏

nynauy commented 1 year ago

Unfortunately, enabling BIT7Z_AUTO_FORMAT through bitdefines.hpp will make src/internal/formatdetect.hpp&cpp empty, with no compilation error. Almost same reason as the 1st point above.

I think you may need to troubleshoot similar problems thoroughly, or just deprecate in-file configuration and focus on CMake options.

rikyoz commented 1 year ago

Unfortunately, enabling BIT7Z_AUTO_FORMAT through bitdefines.hpp will make src/internal/formatdetect.hpp&cpp empty, with no compilation error. Almost same reason as the 1st point above.

I think you may need to troubleshoot similar problems thoroughly

Yep, you're right! I should have managed to fix every missing inclusion of bitdefines.hpp now!

or just deprecate in-file configuration and focus on CMake options.

Yeah, I'm evaluating whether to keep the in-file configuration in the future, but I think I'll keep it for the time being!

rikyoz commented 9 months ago

Fixed in v4.0.0.