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

curl: add compatibility for Amiga and GCC 6.5 #6220

Closed
wants to merge 2 commits into from
Closed

curl: add compatibility for Amiga and GCC 6.5 #6220

wants to merge 2 commits into from

Conversation

OliverUrbann
Copy link
Contributor

Changes are mainly reordering and adding of includes required
to compile with a more recent version of GCC.

@OliverUrbann
curl: add compatibility for Amiga and GCC 6.5
c1ca12a 
Changes are mainly reordering and adding of includes required
to compile with a more recent version of GCC.
@OliverUrbann
Copy link
Contributor Author

I'd also recommend to let @chris-y have a look at it as he made previous Amiga related changes.

@ghost
Copy link

ghost commented Nov 18, 2020 •
edited by ghost

Congratulations 🎉. DeepCode analyzed your code in 2.965 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

chris-y
chris-y reviewed Nov 18, 2020
View changes
include/curl/curl.h Show resolved Hide resolved
@chris-y
Copy link
Contributor

chris-y commented Nov 18, 2020

Can't see any issues here.

@OliverUrbann
Copy link
Contributor Author

OliverUrbann commented Nov 18, 2020 •
edited

I get many of these errors without select.h where fd_set is defined:

CC       vauth/libcurl_la-ntlm_sspi.lo
In file included from ../include/curl/curl.h:3008:0,
                 from ../lib/curl_setup.h:152,
                 from vauth/krb5_gssapi.c:26:
../include/curl/multi.h:159:40: error: unknown type name 'fd_set'
                                        fd_set *read_fd_set,
                                        ^~~~~~

Probably you did not use NTLM, or anything in my configuration is different (clib2 etc.)?

configure: Configured to build curl/libcurl:

  Host setup:       m68k-unknown-amigaos
  Install prefix:   /usr/local
  Compiler:         m68k-amigaos-gcc
   CFLAGS:          -isystem amissl/include -mcrt=clib2 -Werror-implicit-function-declaration -O2 -Wno-system-headers
   CPPFLAGS:
   LDFLAGS:         -LAmiSSL-4.6/AmiSSL/Developer/lib/AmigaOS3
   LIBS:            -lamisslauto -lnet -lm -lc -mcrt=clib2

  curl version:     7.74.0-DEV
  SSL:              enabled (AmiSSL)
  SSH:              no      (--with-{libssh,libssh2})
  zlib:             no      (--with-zlib)
  brotli:           no      (--with-brotli)
  zstd:             no      (--with-zstd)
  GSS-API:          no      (--with-gssapi)
  TLS-SRP:          no      (--enable-tls-srp)
  resolver:         default (--enable-ares / --enable-threaded-resolver)
  IPv6:             no      (--enable-ipv6)
  Unix sockets:     no      (--enable-unix-sockets)
  IDN:              no      (--with-{libidn2,winidn})
  Build libcurl:    Shared=no, Static=yes
  Built-in manual:  enabled
  --libcurl option: enabled (--disable-libcurl-option)
  Verbose errors:   enabled (--disable-verbose)
  Code coverage:    disabled
  SSPI:             no      (--enable-sspi)
  ca cert bundle:   no
  ca cert path:
  ca fallback:
  LDAP:             no      (--enable-ldap / --with-ldap-lib / --with-lber-lib)
  LDAPS:            no      (--enable-ldaps)
  RTSP:             enabled
  RTMP:             no      (--with-librtmp)
  Metalink:         no      (--with-libmetalink)
  PSL:              no      (libpsl not found)
  Alt-svc:          enabled
  HTTP2:            no      (--with-nghttp2)
  HTTP3:            no      (--with-ngtcp2, --with-quiche)
  ECH:              no      (--enable-ech)
  Protocols:        DICT FILE FTP FTPS GOPHER HTTP HTTPS IMAP IMAPS MQTT POP3 POP3S RTSP SMTP SMTPS TELNET TFTP
  Features:         SSL alt-svc

@chris-y
Copy link
Contributor

chris-y commented Nov 18, 2020

Not sure I've ever tried it with NTLM. If it is needed for that then no problem, I just wasn't sure why it wasn't included before!

bagder
bagder reviewed Nov 19, 2020
View changes
configure.ac
@@ -863,27 +863,24 @@ then
])
fi

if test "$HAVE_GETHOSTBYNAME" != "1"
Copy link
Member

Choose a reason for hiding this comment

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

Why remove this check? Doesn't this just make the amiga-specific check now instead get run for everyone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the disadvantage of this change. What I need is that this check is executed even if gethostbyname is already found as there can be multiple sources for that. This check would additionally enable AmiSSL. However, I can try to find a way to execute it only if we know we are compiling it for Amiga but then always.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is now done if no gethostbyname is found and if AmiSSL is requested.

configure.ac Outdated
])
fi
]],[[
gethostbyname("www.dummysite.com");
Copy link
Member

Choose a reason for hiding this comment

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

While here, maybe change this "dummy" name to "example.com" or perhaps just an empty string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just left it like it was but no problem, I can change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see now that www.dummysite.com is also used for Minix, Windows and eCos so I would recommend not to change it.

lib/curl_setup.h Outdated Show resolved Hide resolved
@OliverUrbann OliverUrbann marked this pull request as draft November 19, 2020 19:49
@OliverUrbann
curl: improved AmigaOS related check and pwd.h usage
964f600 
Check for bsdsocket.library is only done if gethostbyname is not
found yet or AmiSSL is requested.  __NO_NET_API is undefined
in source file where required.
@OliverUrbann OliverUrbann marked this pull request as ready for review November 19, 2020 20:57
@OliverUrbann OliverUrbann marked this pull request as draft November 19, 2020 21:33
@OliverUrbann OliverUrbann marked this pull request as ready for review November 19, 2020 21:34
bagder
bagder approved these changes Nov 20, 2020
View changes
@bagder
Copy link
Member

bagder commented Nov 20, 2020

Thanks!

@bagder bagder closed this in 0d16a49 Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chris-y chris-y chris-y left review comments

@bagder bagder bagder approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@OliverUrbann @chris-y @bagder