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.

Sign up for GitHub

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

NetBSD 9.0 build fixes #3422

Merged
merged 3 commits into from
Jun 22, 2020
Merged

NetBSD 9.0 build fixes #3422

merged 3 commits into from
Jun 22, 2020

Conversation

alarixnia
Copy link
Contributor

@alarixnia alarixnia commented Jun 11, 2020 •
edited by mpg
Loading

  • Define _XOPEN_SOURCE in specific files to get definitions of gethostname and sockaddr_storage
  • Add another check for socklen_t's definition. I'm not really happy adding to this since it's quite hacky/messy, but it seems like the least intrusive change I can make for now. (I'm not sure which platforms without socklen_t need special-casing).

Fixes #2310

Backports:

@gilles-peskine-arm
Copy link
Contributor

Thanks! I assume that you've tested on NetBSD, including any relevant older versions.

Please adjust either the author or the signed-off-by to placate the DCO check.

@gilles-peskine-arm gilles-peskine-arm self-assigned this Jun 11, 2020
@gilles-peskine-arm gilles-peskine-arm added component-platform Portability layer and build scripts needs-ci Needs to pass CI tests needs-review Every commit must be reviewed by at least two team members, labels Jun 11, 2020
@gilles-peskine-arm
Copy link
Contributor

Since this could break a “weird” platform that doesn't follow POSIX exactly, we shouldn't backport this.

@gilles-peskine-arm
Copy link
Contributor

Please add a changelog entry file under ChangeLog.d (see ChangeLog for style).

alarixnia added a commit to alarixnia/mbedtls that referenced this pull request Jun 11, 2020
@alarixnia
Add ChangeLog.d entry for Mbed-TLS#3422
1b76520
@alarixnia alarixnia force-pushed the net-sockets-fixes branch from 1b76520 to 508e21c Compare June 11, 2020 13:06
@alarixnia
Copy link
Contributor Author

@gilles-peskine-arm Done.

@mpg mpg removed the needs-work label Jun 19, 2020
mpg
mpg approved these changes Jun 19, 2020
View reviewed changes
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

I can't confirm myself that the changes fix the issue on NetBSD, but I trust you tested that, the changes looks reasonable and unlikely to break anything on other platforms, so I'm approving this PR.

@mpg mpg removed the needs-ci Needs to pass CI tests label Jun 22, 2020
@mpg
Copy link
Contributor

mpg commented Jun 22, 2020

The failure of Mbed OS testing in the pr-merge job is unrelated to this PR (it's caused by changes to the Mbed OS importer).

gilles-peskine-arm
gilles-peskine-arm approved these changes Jun 22, 2020
View reviewed changes
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

The code

ChangeLog.d/bugfix_PR3422.txt
@@ -0,0 +1,2 @@
Bugfix
* Fix building library/net_sockets.c and the ssl_mail_client program on NetBSD. Contributed by Nia Alarie in #3422.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not going to reject the PR for this because it doesn't break any automation and we'll likely do a pass of reformatting when we prepare the release. But for future reference, please wrap changelog lines to 80 columns.

@gilles-peskine-arm gilles-peskine-arm removed the needs-review Every commit must be reviewed by at least two team members, label Jun 22, 2020
@gilles-peskine-arm gilles-peskine-arm merged commit e81dc00 into Mbed-TLS:development Jun 22, 2020
@krytarowski
Copy link

Can we have a new release please with this and #3423 included?

@gilles-peskine-arm
Copy link
Contributor

Both will be in Mbed TLS 2.23.0 which is scheduled to be released today.

This was referenced Aug 13, 2020
Merged
Merged
@gufe44 gufe44 mentioned this pull request Sep 9, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gilles-peskine-arm gilles-peskine-arm gilles-peskine-arm approved these changes

@mpg mpg mpg approved these changes

Assignees

@gilles-peskine-arm gilles-peskine-arm

Labels
component-platform Portability layer and build scripts
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2.14.1: Compilation issue on NetBSD
4 participants
@alarixnia @gilles-peskine-arm @mpg @krytarowski