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

Compiling "threaded-ssl.c" with PThreads4W #13532

Open
gvanem opened this issue May 4, 2024 · 4 comments
Open

Compiling "threaded-ssl.c" with PThreads4W #13532

gvanem opened this issue May 4, 2024 · 4 comments

Comments

@gvanem
Copy link
Contributor

gvanem commented May 4, 2024

I did this

Compiling docs/examples/threaded-ssl.c using POSIX threads for Windows or
PThreads4W, gives me this compile error (from clang-cl):

threaded-ssl.c(68,24): error: operand of type 'pthread_t' (aka '__ptw32_handle_t') 
where arithmetic or pointer type is required
   68 |   ret = (unsigned long)pthread_self();
      |                        ^~~~~~~~~~~~~~

This is caused by pthread_t which is a structure on Pthreads4W.

But with this patch, it compiles:

--- a/docs/examples/threaded-ssl.c 2023-01-05 13:01:10
+++ b/docs/examples/threaded-ssl.c 2024-05-04 09:18:18
@@ -65,7 +65,11 @@
 {
   unsigned long ret;

+#if defined(__PTW32_H)   /* Using 'Pthreads for Windows' */
+  ret = *(unsigned long*) (pthread_self().p);
+#else
   ret = (unsigned long)pthread_self();
+#endif
   return ret;
 }

And the program works.

I expected the following

A successful compile.

curl/libcurl version

WARNING: this libcurl is Debug-enabled, do not use in production

curl 8.8.0-DEV (x86_64-pc-win32) libcurl/8.8.0-DEV (mbedTLS/3.6.0) (OpenSSL/3.3.0) Schannel (BearSSL) 
(rustls-ffi/0.13.0/rustls/0.23.4) zlib/1.3 brotli/1.1.0 zstd/1.5.5 c-ares/1.28.1 WinIDN libssh2/1.10.1_DEV nghttp2/1.61.0
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps ipfs ipns ldap ldaps mqtt pop3 pop3s rtsp scp sftp 
smb smbs smtp smtps telnet tftp ws wss
Features: alt-svc AsynchDNS brotli Debug HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz MultiSSL 
NTLM SPNEGO SSL SSPI threadsafe TLS-SRP TrackMemory UnixSockets zstd

operating system

Win-10.

@bagder
Copy link
Member

bagder commented May 27, 2024

I think that proposal introduces a hideous piece of code I rather not have in an example we provide showing off libcurl with.

@jay
Copy link
Member

jay commented May 31, 2024

I think that proposal introduces a hideous piece of code I rather not have in an example we provide showing off libcurl with.

Seems harsh. POSIX allows pthread_t to be opaque so the way we cast it to long for thread id will not always work, like for PThreads4W as we can see here. How about a comment that explains this situation:

diff --git a/docs/examples/threaded-ssl.c b/docs/examples/threaded-ssl.c
index f58e447..4feea80 100644
--- a/docs/examples/threaded-ssl.c
+++ b/docs/examples/threaded-ssl.c
@@ -65,6 +65,8 @@ static unsigned long thread_id(void)
 {
   unsigned long ret;

+  /* How to obtain the thread ID will vary depending on the pthreads library
+     you are using. This example assumes pthread_self returns the ID. */
   ret = (unsigned long)pthread_self();
   return ret;
 }

@gvanem
Copy link
Contributor Author

gvanem commented Jun 1, 2024

How about a comment that explains this situation:

A comment wont fix the compile error.

@jay
Copy link
Member

jay commented Jun 1, 2024

A comment wont fix the compile error.

Yeah but it is an example, it's not meant to address all cases. It is not correct to cast pthread_self() to unsigned long. It is not correct to cast pthread_self().p to unsigned long either.

jay added a commit to jay/curl that referenced this issue Jun 1, 2024
- Add a comment explaining that pthread_self(), which returns pthread_t,
  is not necessarily an integer thread ID.

pthread_t type varies depending on the pthread library. For some pthread
libraries, such as PThreads4W, it is a struct and not actual thread ID.

Rather than update the example to handle every pthread implementation of
thread ID we now explain that it varies depending on the library.

Bug: curl#13532
Reported-by: Gisle Vanem

Closes #xxxx
jay added a commit to jay/curl that referenced this issue Jun 1, 2024
- Add a comment explaining that pthread_self(), which returns pthread_t,
  is not necessarily an integer thread ID.

pthread_t type varies depending on the pthread library. For some pthread
libraries, such as PThreads4W, it is a struct and not integer thread ID.

Rather than update the example to handle every pthread implementation of
thread ID we now explain that it varies depending on the library.

Bug: curl#13532
Reported-by: Gisle Vanem

Closes #xxxx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants