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

Fix building on MSVC #20

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

FtZPetruska
Copy link

While working on #19, I noticed building on MSVC due to the following problems:

  • __attribute__((unused)) does not exist, this is only used in some files from unzip.
  • VLAs are not supported, this is used once in statesaver.cpp:553.

For the __attribute__ issue, I just added a simple check for MSVC:

#ifdef _MSC_VER
#define ATTRIBUTE_UNUSED
#else
#define ATTRIBUTE_UNUSED __attribute__((unused))
#endif

For the VLA, I simply replaced it with a std::string as it provides a convenient way to replicate the behaviour.

- `__attribute__((unused))` does not exist.
- VLAs are not supported, use a std::string instead.
@CasualPokePlayer
Copy link
Member

CasualPokePlayer commented Jun 4, 2023

Unused parameters probably are better off using the standard way indicate them, with (void)unused_variable;.

I'm not sure if std::string is appropriate here, as this is all raw data rather than some string. A std::unique_ptr<char[]> may be better here.

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

Successfully merging this pull request may close these issues.

2 participants