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 32-bit timestamp issue in public API #735 #1220

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

doorsdown
Copy link
Contributor

libssh2 issue #735 reported that timestamp fields in LIBSSH2_SFTP_ATTRIBUTES struct were limited to 32 bits on non 64 bit machines

Closes: #735

…TP_ATTRIBUTES struct were limited to 32 bits on non 64 bit machines
@vszakats
Copy link
Member

Are we comfortable with breaking the ABI for this?

If so, this will need an SO version bump. Then there is a bunch
of other things that might use a breaking upgrade if we're doing this.

@willco007
Copy link
Member

What reasonable options do we have that won't?

@vszakats
Copy link
Member

Introducing new API variants that work with the new types.

Perhaps extending this struct with new 64-bit values (while
keeping the existing ones) might also work, I'm not sure.

@willco007
Copy link
Member

Adding new/duplicate API seems like not a great solution to a long term problem. Extending the struct would work, but again it's a bit of a fragile fix for a long term issue. I'd probably be OK breaking ABI but I could be convinced to keep it. :)

@vszakats
Copy link
Member

vszakats commented Nov 15, 2023

I'm not for or against an ABI break, but in case we do it, it's a bigger deal;
It needs coordination and completing the At next SONAME bump list in
docs/TODO.

Also, before doing an SO bump, it'd be useful IMO to do a non-breaking
release with the fixes since 1.11.0, including the pending OpenSSL 3 PR: #1207.

@vszakats vszakats changed the title libssh2 issue #735 32bit timestamp issue fix 32-bit timestamp issue in public API #735 Nov 15, 2023
@vszakats
Copy link
Member

vszakats commented Dec 17, 2023

I'm thinking putting this change behind an internal macro (e.g. __LIBSSH2_ABI_BREAK_NEXT) reserved for debug builds, alongside a CI test. This would allow to merge this early (now), and potentially allow to prepare new ABI breaking changes in the source, and release/enable them when everything is ready.

Anyone opposing this?

Edit: For documentation the pages could be duplicated into a modified, not distributed copy. Perhaps.

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

Successfully merging this pull request may close these issues.

libssh2_sftp_stat_ex() uses "long" for times
3 participants