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

Jump to bottom

Classic Mac OS: fix socklen_t, curl_off_t, long long #10049

Closed
wants to merge 6 commits into from
Closed

Classic Mac OS: fix socklen_t, curl_off_t, long long #10049

wants to merge 6 commits into from

Conversation

ryandesign
Copy link
Contributor

Change CURL_TYPEOF_CURL_SOCKLEN_T from int to unsigned int when __MWERKS__ is defined (i.e. Metrowerks compilers i.e. CodeWarrior).

This fixes a build failure when compiling for classic Mac OS with GUSI with CodeWarrior compilers.

The error was:

#----------------------------------------------------------
### MWC68K Compiler Error:
#	if(getsockname(sockfd, (struct sockaddr *) &add, &size) < 0) {
#	                                                      ^
# cannot convert
# 'int *' to
# 'unsigned int *'
#----------------------------------------------------------
File "connect.c"; Line 462
#----------------------------------------------------------
### MWC68K Compiler Error:
#	if(0 != getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (void *)&err, &errSize))
#	                                                                       ^
# cannot convert
# 'int *' to
# 'unsigned int *'
#----------------------------------------------------------
File "connect.c"; Line 535
#----------------------------------------------------------

It's unclear to me what system was envisioned when the __MWERKS__ block was originally added in 2008 in 3ac6929. Metrowerks CodeWarrior was originally a classic Mac OS compiler in the 90s but, later, versions for BeOS, Windows, and Mac OS X existed, and it could cross-compile code for other systems as well. The Metrowerks company no longer exists today but CodeWarrior still does and is now used for embedded systems. I'm not certain that all of the systems CodeWarrior supports or supported will have the same values for the constants being defined here.

For example, POSIX says socklen_t should be unsigned.

  • GUSI for classic Mac OS started using socklen_t in version 2. It is unsigned in the earliest version still posted on the site (2.1.3 from 2000) and in the last version (2.2.3 from 2002).
  • Mac OS X first used socklen_t in version 10.3 (2003) and it was signed. By Mac OS X 10.4 (2005) it was unsigned.
  • BeOS (1995-2001) apparently used a signed socklen_t at least for a time according to a quick Internet search. Don't know what Haiku (the reimplementation of BeOS) does.
  • No idea what Windows or embedded systems do.

I'm not sure how these potential differences should be handled here.

@jay
Copy link
Member

jay commented Dec 7, 2022 •
edited

I'm not certain that all of the systems CodeWarrior supports or supported will have the same values for the constants being defined here.

Maybe we should stop using __MWERKS__ or add more qualifiers? I think we can assume that section was probably for old macs.

@jay jay added the build label Dec 7, 2022
@dfandrich
Copy link
Contributor

FWIW, Haiku defines socklen_t as uint32_t in sys/socket.h

@ryandesign
system.h: change "__MWERKS__" block to "macintosh"
87e9a1a 
Change "__MWERKS__" to "macintosh". When this block was originally added
in 3ac6929 it was probably intended to handle classic Mac OS since the
previous classic Mac OS build procedure for curl (which was removed in
bf327a9) used Metrowerks CodeWarrior.

But there are other classic Mac OS compilers, such as the MPW compilers,
that were not handled by this case. For classic Mac OS,
CURL_TYPEOF_CURL_SOCKLEN_T needs to match what's provided by the
third-party GUSI library, which does not vary by compiler.

Meanwhile CodeWarrior works on platforms other than classic Mac OS, and
they may need different definitions. Separate blocks could be added
later for any of those platforms that curl doesn't already support.
@ryandesign
system.h: make socklen_t unsigned for classic Mac OS
7da05ff 
Change CURL_TYPEOF_CURL_SOCKLEN_T from "int" to "unsigned int" when
"macintosh" is defined.

This fixes a build failure when compiling for classic Mac OS with GUSI.
@ryandesign
system.h: handle Mac compilers without long long
30985bd 
Not all classic Mac OS compilers support the "long long" data type.
Include the <ConditionalMacros.h> system header which defines
"TYPE_LONGLONG" to 0 or 1 to indicate whether the current version of the
current compiler supports "long long" and define the curl_off_t type /
format / suffix based on that.
@ryandesign
config-mac: define HAVE_LONGLONG if supported
825d010 
Not all classic Mac OS compilers support the "long long" data type.
Include the <ConditionalMacros.h> system header which defines
"TYPE_LONGLONG" to 0 or 1 to indicate whether the current version of the
current compiler supports "long long" and define HAVE_LONGLONG if so.
@ryandesign
config-mac: define SIZEOF_CURL_OFF_T
31b037e 
Define SIZEOF_CURL_OFF_T to 4 or 8 for classic Mac OS depending on
whether the compiler supports "long long".
@ryandesign
Copy link
Contributor Author

Maybe we should stop using __MWERKS__ or add more qualifiers?

Perhaps so. Several of the other top-level conditions in that file are already about operating systems (e.g. _WIN32_WCE, __VMS, __OS400__), not compilers.

I think we can assume that section was probably for old macs.

Probably. I've rewritten the PR for this. I've divided it into a few hopefully sensibly distinct changes.

@ryandesign ryandesign changed the title system.h: socklen_t is unsigned for __MWERKS__ Classic Mac OS: fix socklen_t, curl_off_t, long long Dec 7, 2022
@ryandesign ryandesign marked this pull request as ready for review December 7, 2022 17:22
@jay
Copy link
Member

jay commented Dec 7, 2022

Is SIZEOF_LONG defined for you? I don't see it in config-mac and there are a few places in the library where it's evaluated

lib/mprintf.c:#if (SIZEOF_CURL_OFF_T > SIZEOF_LONG)
lib/mprintf.c:#if (SIZEOF_SIZE_T > SIZEOF_LONG)
lib/mprintf.c:#if (SIZEOF_CURL_OFF_T > SIZEOF_LONG)

@ryandesign
Copy link
Contributor Author

Good point! I've added a commit to define SIZEOF_LONG.

@bagder bagder closed this in ac45548 Dec 9, 2022
@bagder
Copy link
Member

bagder commented Dec 9, 2022

Thanks!

@ryandesign ryandesign deleted the patch-1 branch December 9, 2022 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bagder bagder bagder approved these changes

Assignees
No one assigned
Labels
build
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ryandesign @jay @dfandrich @bagder