[openssl/openssl] a2c74d: threads_win: fix build error with mingw64

[Georgi Valkov]

  Branch: refs/heads/master
  Home:   https://github.com/openssl/openssl
  Commit: a2c74d7af66e6eff9b4355b27e760e8517746f08
      https://github.com/openssl/openssl/commit/a2c74d7af66e6eff9b4355b27e760e8517746f08
  Author: Georgi Valkov <gvalkov at gmail.com>
  Date:   2024-07-17 (Wed, 17 Jul 2024)

  Changed paths:
    M crypto/threads_win.c

  Log Message:
  -----------
  threads_win: fix build error with mingw64

This fixes a build error regression on mingw64 introduced by me in
16beec98d26644b96d57bd8da477166d0bc7d05c

In get_hold_current_qp, uint32_t variables were improperly
used to hold the value of reader_idx, which is defined as long int.
So I used CRYPTO_atomic_load_int, where a comment states
On Windows, LONG is always the same size as int

There is a size confusion, because
Win32 VC x86/x64: LONG, long, long int are 32 bit
MingW-W64: LONG, long, long int are 32 bit
cygwin64: LONG is 32 bit, long, long int are 64 bit

Fix:
- define reader_idx as uint32_t
- edit misleading comment, to clarify:
On Windows, LONG (but not long) is always the same size as int.

Fixes the following build error, reported in [1].
crypto/threads_win.c: In function 'get_hold_current_qp':
crypto/threads_win.c:184:32: error: passing argument 1 of 'CRYPTO_atomic_load_int' from incompatible pointer type [-Wincompatible-pointer-types]
  184 |         CRYPTO_atomic_load_int(&lock->reader_idx, (int *)&qp_idx,
      |                                ^~~~~~~~~~~~~~~~~
      |                                |
      |                                volatile long int *

[1] https://github.com/openssl/openssl/pull/24405#issuecomment-2211602282

Signed-off-by: Georgi Valkov <gvalkov at gmail.com>

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24803)


  Commit: ce6b2f98263712b2ccb4559117cbd480c552894b
      https://github.com/openssl/openssl/commit/ce6b2f98263712b2ccb4559117cbd480c552894b
  Author: Georgi Valkov <gvalkov at gmail.com>
  Date:   2024-07-17 (Wed, 17 Jul 2024)

  Changed paths:
    M crypto/threads_pthread.c
    M crypto/threads_win.c

  Log Message:
  -----------
  threads_pthread, threads_win: improve code consistency

Improve code consistency between threads_pthread.c and threads_win.c
threads_pthread.c has good comments, let's copy them to threads_win.c
In many places uint64_t or LONG int was used, and assignments were
performed between variables with different sizes.
Unify the code to use uint32_t. In 32 bit architectures it is easier
to perform 32 bit atomic operations. The size is large enough to hold
the list of operations.
Fix result of atomic_or_uint_nv improperly casted to int *
instead of int.

Note:
In general size_t should be preferred for size and index, due to its
descriptive name, however it is more convenient to use uint32_t for
consistency between platforms and atomic calls.

READER_COUNT and ID_VAL return results that fit 32 bit. Cast them to
uint32_t to save a few CPU cycles, since they are used in 32 bit
operations anyway.

TODO:
In struct rcu_lock_st, qp_group can be moved before id_ctr
for better alignment, which would save 8 bytes.

allocate_new_qp_group has a parameter count of type int.
Signed values should be avoided as size or index.
It is better to use unsigned, e.g uint32_t, even though
internally this is assigned to a uint32_t variable.

READER_SIZE is 16 in threads_pthread.c, and 32 in threads_win.c
Using a common size for consistency should be prefered.

Signed-off-by: Georgi Valkov <gvalkov at gmail.com>

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24803)


Compare: https://github.com/openssl/openssl/compare/29bbe7d0086a...ce6b2f982637

To unsubscribe from these emails, change your notification settings at https://github.com/openssl/openssl/settings/notifications