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

strncpy filename_size #273

Merged
merged 1 commit into from
Aug 5, 2022
Merged

strncpy filename_size #273

merged 1 commit into from
Aug 5, 2022

Conversation

kuba--
Copy link
Owner

@kuba-- kuba-- commented Aug 2, 2022

@kuba-- kuba-- linked an issue Aug 2, 2022 that may be closed by this pull request
Copy link

@Limaaron Limaaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't solve the problem since the maximum path length is 260 symbols.
MAX_PATH restrictions apply until Windows 10 version 1607

as a solution I can suggest this code:

/* Note: MAX_PATH restrictions apply until Windows 10 version 1607
  */
#if defined(_WIN32)
#   define MAX_FILE_PATH 256
#else
#   define MAX_FILE_PATH 512
#endif

enum {
  /* Note: These enums can be reduced as needed to save memory or stack space -
     they are pretty conservative. */
  MZ_ZIP_MAX_IO_BUF_SIZE = 8 * 1024,
  MZ_ZIP_MAX_ARCHIVE_FILENAME_SIZE = MAX_FILE_PATH,
  MZ_ZIP_MAX_ARCHIVE_FILE_COMMENT_SIZE = 512
};

@kuba--
Copy link
Owner Author

kuba-- commented Aug 3, 2022

This won't solve the problem since the maximum path length is 260 symbols. MAX_PATH restrictions apply until Windows 10 version 1607

as a solution I can suggest this code:

/* Note: MAX_PATH restrictions apply until Windows 10 version 1607
  */
#if defined(_WIN32)
#   define MAX_FILE_PATH 256
#else
#   define MAX_FILE_PATH 512
#endif

enum {
  /* Note: These enums can be reduced as needed to save memory or stack space -
     they are pretty conservative. */
  MZ_ZIP_MAX_IO_BUF_SIZE = 8 * 1024,
  MZ_ZIP_MAX_ARCHIVE_FILENAME_SIZE = MAX_FILE_PATH,
  MZ_ZIP_MAX_ARCHIVE_FILE_COMMENT_SIZE = 512
};

But this is totally not related to the issue which you came across.
MAX_PATH on windows < 10 is a one thing, but you got an error related to strncpy which potentially can copy more than array size.

@kuba--
Copy link
Owner Author

kuba-- commented Aug 3, 2022

...Moreover lets take a look step by step:

we need a minimum of filename_size (which by default is MZ_ZIP_MAX_ARCHIVE_FILENAME_SIZE (512)) and MAX_PATH - dirlen.

  if (filename_size > MAX_PATH - dirlen) {
    filename_size = MAX_PATH - dirlen;
  }

MAX_PATH is defined in zip.h as follows:

#ifndef MAX_PATH
#define MAX_PATH 1024 /* # chars in a path name including NULL */
#endif

So if you are on windows where MAX_PATH is defined as 256, then it should work, because dirlen cannot be > MAX_PATH (because function returns ZIP_EINVENTNAME if (dirlen + 1 > MAX_PATH) )

@Limaaron
Copy link

Limaaron commented Aug 3, 2022

MAX_PATH is defined in zip.h as follows:

#ifndef MAX_PATH
#define MAX_PATH 1024 /* # chars in a path name including NULL */
#endif

MAX_PATH is declared inside minwindef.h.
I think that's what causes the problem.

@kuba--
Copy link
Owner Author

kuba-- commented Aug 3, 2022

MAX_PATH is defined in zip.h as follows:

#ifndef MAX_PATH
#define MAX_PATH 1024 /* # chars in a path name including NULL */
#endif

MAX_PATH is declared inside minwindef.h. I think that's what causes the problem.

What problem exactly?
In this case (https://github.com/tpn/winsdk-10/blob/master/Include/10.0.16299.0/shared/minwindef.h#L60), just include zip.h after windows' headers.

@Limaaron
Copy link

Limaaron commented Aug 3, 2022

this did not solve the problem, I tried to build it as an independent library and the ide tells me that MAX_PATH is defined in minwindef.h.

@Limaaron
Copy link

Limaaron commented Aug 3, 2022

it seems strange to me that when compiling without the -DCMAKE_BUILD_TYPE=Release, it does not give such an error.

@kuba-- kuba-- merged commit 114d9d4 into master Aug 5, 2022
@kuba-- kuba-- deleted the fix-272 branch August 5, 2022 10:31
thabetx pushed a commit to Symbyo360/zip that referenced this pull request Sep 7, 2023
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.

Error when building the release version with MinGW
2 participants