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

system.h: drop compilers lacking 64-bit integer type (Windows/MS-DOS) #15957

Closed
wants to merge 8 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Jan 9, 2025

  • DJGPP 1.x (including __GO32__) (MS-DOS)
    DJGPP 2.x support remains unchanged.
  • Salford C (Windows)
  • Turbo C (Windows 16-bit)
  • Borland C++ < 5.2 (Windows 16-bit?)
  • Pelles C < 2.8 (Windows)

These targets mapped curl_off_t to long. On Windows and MS-DOS
long is always 32-bit.

curl requires C compilers supporting 64-bit curl_off_t type since
8356826 #10597 (v8.0.0).

Also: drop remaining __GO32__ and Salford C guards.


w/o whitespace: https://github.com/curl/curl/pull/15957/files?w=1

@vszakats vszakats changed the title include: drop support for 32-bit compilers include: drop support for compilers lacking 64-bit type Jan 9, 2025
@vszakats vszakats changed the title include: drop support for compilers lacking 64-bit type include: drop support for compilers lacking 64-bit integer Jan 9, 2025
@dfandrich
Copy link
Contributor

dfandrich commented Jan 9, 2025 via email

@vszakats
Copy link
Member Author

vszakats commented Jan 9, 2025

32-bit compilers are supposed to still be supported as long as they supply a 64-bit integer type. Do none of these do that? If so, the description could be clarified.

Indeed, fixed it after realizing it!

I have no way to actually test these platforms, but it's more than unlikely
these MS-DOS and Windows compilers would set long to a 64-bit type.
It seems safe to delete them, unless I'm missing something obvious.

@dfandrich
Copy link
Contributor

dfandrich commented Jan 9, 2025 via email

@vszakats
Copy link
Member Author

vszakats commented Jan 9, 2025

At least one DOS compiler I'm aware of (Watcom C) supports a 64-bit type and it wouldn't surprise me if others did, too. AIUI it doesn't have to be long, just some 64-bit type that curl can use. At least one person is still producing recent curl builds under DOS, so I wouldn't be too hasty to remove that support.

This doesn't remove MS-DOS support. DJGPP still builds in CI.

It drops support for DJGPP v1, using long. It must be an early
version. For a long time v2 has been the DJGPP version used
(last 20 years). This patch doesn't change anything with v2.

__GO32__ seems to be some kind of DJGPP mode, possibly
for v1. Looks niche or died out. In my other FLOSS project I never
met with this macro since 1999.

As for Watcom, there was no mention of __WATCOMC__ in
system.h or in the source. This PR also doesn't change things
Watcom related. (It'd be interesting to see how curl is built with
Watcom.)

What's deleted here is ancient or defunct compilers and versions.

@vszakats
Copy link
Member Author

vszakats commented Jan 9, 2025

As for Borland C++, 5.5 was the first free version capable of
building for 32-bit Windows. Anything earlier is very niche and unlikely
anyone uses it. Pelles C has been dead for a decade now. My last
record was about v7.x. This patch deletes support for v2.8, which
was probably a decade+ preceding that. Both used long which
was not a 64-bit type in either compiler.

@rsbeckerca
Copy link
Contributor

I am a little confused about this issue. Does this mean that support for long long is fine or does it mean that int or long must be 64-bit.

@rsbeckerca
Copy link
Contributor

I am a little confused about this issue. Does this mean that support for long long is fine or does it mean that int or long must be 64-bit.

I should clarify that I have two curl builds, 32-bit and 64-bit. Both are required. long long is 64-bit in both.

@dfandrich
Copy link
Contributor

dfandrich commented Jan 9, 2025 via email

@bagder
Copy link
Member

bagder commented Jan 9, 2025

long is also 32 bit on 64 bit Windows and possibly some other odd platforms.

As long (pun not intended) as there is a 64 bit type available, we should be good.

@vszakats
Copy link
Member Author

vszakats commented Jan 9, 2025

system.h deals with mapping curl_off_t to 64-bit types.
According to 8356826 curl requires this type to be 64-bit.

For targets deleted in this PR, this mapping looked like:

#define CURL_TYPEOF_CURL_OFF_T     long

As opposed to targets with 64-bit type support, where it's one of these:

#define CURL_TYPEOF_CURL_OFF_T     long long
#define CURL_TYPEOF_CURL_OFF_T     __int64

I understand these platform may theoretically provide 64-bit
types. But where are those mapped or used, if they do exist?

I don't think these ancient compilers provided 64-bit types,
but regardless, existing code mapped them to long, which
was never a 64-bit type for either Windows or MS-DOS, even
if the compiler supported targeting 64-bit Windows. (though
none of the deleted ones did.)

All deleted targets are exclusively Windows and MS-DOS
compilers, where long is always 32-bit, thus not meeting
the 64-bit curl_off_t requirement.

What am I missing?

@vszakats vszakats changed the title include: drop support for compilers lacking 64-bit integer include: drop support for Windows/MS-DOS compilers lacking 64-bit integer Jan 9, 2025
@vszakats vszakats changed the title include: drop support for Windows/MS-DOS compilers lacking 64-bit integer include: drop support for compilers lacking 64-bit integer (Windows/MS-DOS) Jan 9, 2025
@vszakats vszakats changed the title include: drop support for compilers lacking 64-bit integer (Windows/MS-DOS) include: drop compilers lacking 64-bit integer type (Windows/MS-DOS) Jan 10, 2025
@vszakats vszakats force-pushed the include-drop-32-bit-platforms branch from 989be21 to 5609360 Compare January 10, 2025 19:15
@vszakats vszakats changed the title include: drop compilers lacking 64-bit integer type (Windows/MS-DOS) include/system.h: drop compilers lacking 64-bit integer type (Windows/MS-DOS) Jan 10, 2025
@vszakats vszakats changed the title include/system.h: drop compilers lacking 64-bit integer type (Windows/MS-DOS) system.h: drop compilers lacking 64-bit integer type (Windows/MS-DOS) Jan 10, 2025
@rsbeckerca
Copy link
Contributor

I am not sure but I think #ifdef __LP64 is portable (or nearly portable) or 64- vs. 32-bit builds.

@vszakats vszakats closed this in ccf43ce Jan 10, 2025
@vszakats vszakats deleted the include-drop-32-bit-platforms branch January 10, 2025 21:47
@vszakats
Copy link
Member Author

I am not sure but I think #ifdef __LP64 is portable (or nearly portable) or 64- vs. 32-bit builds.

Moved that update to #15966 and did the same for __IBMC__. I ended up keeping the _LP64
reference in a comment. But, the branch it guarded was identical to the default one, and could be
deduped. Let me know if I missed something.

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

Successfully merging this pull request may close these issues.

5 participants