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 file offest on Windows and change offset increment #563

Merged
merged 2 commits into from
Aug 7, 2023

Conversation

JacobBarthelmeh
Copy link
Contributor

Reported via direct email. On Windows the high offset was not getting set with SFTP leading to not stopping when writing files larger than max word32 size (over 4G). When fixing that I also changed the assign function to not depend on overflow (undefined behavior) for incrementing the high offset value.

src/internal.c Outdated
@@ -13217,12 +13217,20 @@ int wolfSSH_oct2dec(WOLFSSH* ssh, byte* oct, word32 octSz)
}


/* not using UINT_MAX here because word32 should always be 32 bits where an
* int type can change depending on platform */
#define WS_WORD32_MAX_SZ 4294967295U
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it too ugly to use something like #define WS_WORD32_MAX_SZ (word32)((1ULL << (sizeof(word32)*4)) - 1)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something like that would work. I get the idea, doing a bitshift to larger than the max size minus 1. Will make the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking for WOLFSSL_BIT_SIZE (8) I see we have WOLFSSL_MAX_32BIT (0xffffffU), which is what I tried to find initially. Will try to use that instead of recreating it.

dgarske
dgarske previously approved these changes Aug 7, 2023
@dgarske dgarske merged commit e089f2d into wolfSSL:master Aug 7, 2023
2 checks passed
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.

3 participants