Skip to content

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Apr 25, 2023

Unless I'm missing something, it looks like libssh2.h has been using
libssh2_int64_t unconditionally since at least 2010-04-17 when
libssh2_scp_send64() landed via commit
be9ee70.

This makes it redundant to detect HAVE_LONGLONG to fallback to a
32-bit scpRecv_size in libssh2_priv.h. Then deal with possible
combinations of this flag and strtoll() options, which was
error-prone.

Instead, assume in libssh2_priv.h that we have libssh2_int64_t, and
use it always.

For MSVC, this means _MSC_VER 1310 (from year 2003) is now
required. Based on the above, this was already so before this patch.

If there happens to be no 64-bit strtoll() detected, fall back to the
32-bit strtol() (this should never happen with MSVC, and probably
neither with any other reasonably modern toolchain.)

Also make sure to set HAVE_STRTOI64 for older, non-CMake, MSVC builds
(e.g. Makefile.mk or NMakefile ones).

Closes #1002

@vszakats vszakats changed the title libssh2_priv.h: make scpRecv_size logic more readable libssh2_priv.h: assume HAVE_LONGLONG Apr 26, 2023
Unless I'm missing something, it looks like `libssh2.h` has been using
`libssh2_int64_t` unconditionally since at least 2010-04-17 when
`libssh2_scp_send64()` landed via commit
be9ee70.

This makes it redundant to detect `HAVE_LONGLONG` to fallback to a
32-bit `scpRecv_size` in `libssh2_priv.h`. Then deal with possible
combinations of this flag and `strtoll()` options, which was error-prone.

Instead, assume in `libssh2_priv.h` that we have `libssh2_int64_t`, and
use it always.

For MSVC, this means `_MSC_VER` `1310` (from year 2003) is now
required. Based on the above, this was already so before this patch.

If there happens to be no 64-bit `strtoll()` detected, fall back to the
32-bit `strtol()` (this should never happen with MSVC, and probably
neither with any other reasonably modern toolchain.)

Also make sure to set `HAVE_STRTOI64` for older, non-CMake, MSVC
builds (e.g. `Makefile.mk` or `NMakefile` ones).

Closes libssh2#1002
@vszakats vszakats closed this in 5db836b Apr 26, 2023
@vszakats vszakats deleted the untangle-int64 branch April 26, 2023 16:49
vszakats added a commit that referenced this pull request May 31, 2023
E.g. on 32-bit Linux. Issue revealed after adding i386 Linux CI build
in abdf40c #1057.

```
/home/runner/work/libssh2/libssh2/src/scp.c: In function 'scp_recv':
/home/runner/work/libssh2/libssh2/src/scp.c:765:23: error: conversion from 'libssh2_int64_t' {aka 'long long int'} to '__off_t' {aka 'long int'} may change value [-Werror=conversion]
  765 |         sb->st_size = session->scpRecv_size;
      |                       ^~~~~~~
```
Ref: https://github.com/libssh2/libssh2/actions/runs/5126803482/jobs/9221746299?pr=1054#step:12:51

Regression from 5db836b #1002
Closes #1060
lampmanyao pushed a commit to lampmanyao/libssh2 that referenced this pull request Jul 16, 2023
E.g. on 32-bit Linux. Issue revealed after adding i386 Linux CI build
in abdf40c libssh2#1057.

```
/home/runner/work/libssh2/libssh2/src/scp.c: In function 'scp_recv':
/home/runner/work/libssh2/libssh2/src/scp.c:765:23: error: conversion from 'libssh2_int64_t' {aka 'long long int'} to '__off_t' {aka 'long int'} may change value [-Werror=conversion]
  765 |         sb->st_size = session->scpRecv_size;
      |                       ^~~~~~~
```
Ref: https://github.com/libssh2/libssh2/actions/runs/5126803482/jobs/9221746299?pr=1054#step:12:51

Regression from 5db836b libssh2#1002
Closes libssh2#1060
agreppin pushed a commit to agreppin/libssh2 that referenced this pull request Jul 14, 2024
Unless I'm missing something, it looks like `libssh2.h` has been using
`libssh2_int64_t` unconditionally since at least 2010-04-17 when
`libssh2_scp_send64()` landed via commit
be9ee70.

This makes it redundant to detect `HAVE_LONGLONG` to fallback to a
32-bit `scpRecv_size` in `libssh2_priv.h`. Then deal with possible
combinations of this flag and `strtoll()` options, which was
error-prone.

Instead, assume in `libssh2_priv.h` that we have `libssh2_int64_t`, and
use it always.

For MSVC, this means `_MSC_VER` `1310` (from year 2003) is now
required. Based on the above, this was already so before this patch.

If there happens to be no 64-bit `strtoll()` detected, fall back to the
32-bit `strtol()` (this should never happen with MSVC, and probably
neither with any other reasonably modern toolchain.)

Also make sure to set `HAVE_STRTOI64` for older, non-CMake, MSVC builds
(e.g. `Makefile.mk` or `NMakefile` ones).

Closes libssh2#1002
agreppin pushed a commit to agreppin/libssh2 that referenced this pull request Jul 14, 2024
E.g. on 32-bit Linux. Issue revealed after adding i386 Linux CI build
in abdf40c libssh2#1057.

```
/home/runner/work/libssh2/libssh2/src/scp.c: In function 'scp_recv':
/home/runner/work/libssh2/libssh2/src/scp.c:765:23: error: conversion from 'libssh2_int64_t' {aka 'long long int'} to '__off_t' {aka 'long int'} may change value [-Werror=conversion]
  765 |         sb->st_size = session->scpRecv_size;
      |                       ^~~~~~~
```
Ref: https://github.com/libssh2/libssh2/actions/runs/5126803482/jobs/9221746299?pr=1054#step:12:51

Regression from 5db836b libssh2#1002
Closes libssh2#1060
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.

1 participant