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

amigaos: fix threaded resolver on AmigaOS 4.x #9265

Closed
wants to merge 1 commit into from

Conversation

Futaura
Copy link
Contributor

@Futaura Futaura commented Aug 7, 2022

Replace ip4 resolution function on AmigaOS 4.x, as it requires
runtime feature detection and extra code to make it thread safe.

#include <proto/bsdsocket.h>

static struct SocketIFace *__CurlISocket = NULL;
static uint32 SocketFeatures = 0;
Copy link
Member

@bagder bagder Aug 8, 2022

Choose a reason for hiding this comment

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

statics really? Are you sure these are thread-safe?

Copy link
Contributor Author

@Futaura Futaura Aug 8, 2022

Choose a reason for hiding this comment

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

I'm with you on the statics, but this is the only way I could think of, without forcing unnecessary extra overhead on every resolve. Normally, I'd rely on the global ISocket variable which the application will have (if using AmiSSL), but since I need to enable the SBTC_CAN_SHARE_LIBRARY_BASES on it to make it thread safe, I didn't want to interfere with that, or indeed allow the application to interfere with libcurl by switching that feature off!

Curl_amiga_init() isn't thread safe by itself, when it comes to initialising __CurlISocket, but as long as GLOBAL_INIT_IS_THREADSAFE is defined it will be thread safe, as it will be on AmigaOS 4.x builds - that should be enough, right? Once initialised, the functionality is completely thread safe (the old code wasn't as it relied on gethostbyname().

This code is going to get reused for enabling getaddrinfo() support to in the future, but that function appears to be broken in the AmigaOS 4.x tcp/ip stack at the moment!

@bagder
Copy link
Member

bagder commented Aug 9, 2022

This PR appears to have merge conflicts now. Can you please rebase and force-push it?

@bagder
Copy link
Member

bagder commented Aug 9, 2022

It still says "This branch cannot be rebased due to conflicts". I really did mean rebase and force-push...

Replace ip4 resolution function on AmigaOS 4.x, as it requires
runtime feature detection and extra code to make it thread safe.
@Futaura Futaura force-pushed the amigaos4_threaded_resolver branch from abc04b6 to 6f00392 Compare Aug 9, 2022
@Futaura
Copy link
Contributor Author

Futaura commented Aug 9, 2022

Sorry - was assuming the buttons on the GitHub website would help me out, but seemed to created a bigger mess! Having booted up linux, hopefully my first attempt at a rebase is correct, but please let me know if not. Thanks.

bagder
bagder approved these changes Aug 10, 2022
@bagder bagder closed this in 55a138a Aug 10, 2022
@bagder
Copy link
Member

bagder commented Aug 10, 2022

Thanks!

@Futaura Futaura deleted the amigaos4_threaded_resolver branch Aug 10, 2022
@gvanem
Copy link
Contributor

gvanem commented Aug 13, 2022

Real retro stuff: curl on Amiga. (just a test of HTML and web-fonts).

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.

None yet

3 participants