Skip to content

Detect accept4() on specific versions of various platforms#294

Merged
zuiderkwast merged 3 commits into
valkey-io:unstablefrom
panjf2000:os-macros
Apr 25, 2024
Merged

Detect accept4() on specific versions of various platforms#294
zuiderkwast merged 3 commits into
valkey-io:unstablefrom
panjf2000:os-macros

Conversation

@panjf2000

@panjf2000 panjf2000 commented Apr 11, 2024

Copy link
Copy Markdown
Contributor

This PR has mainly done three things:

  1. Enable accept4() on DragonFlyBSD 4.3+
  2. Fix the failures of determining the presence of accept4() due to the missing <sys/param.h> on two OSs: NetBSD, OpenBSD
  3. Drop the support of FreeBSD <10.0 for valkey

References

Release notes:
Detect accept4() on NetBSD, OpenBSD and DragonFlyBSD.

Signed-off-by: Andy Pan <i@andypan.me>
@panjf2000

Copy link
Copy Markdown
Contributor Author

@madolson @zuiderkwast Got a minute for this?

Signed-off-by: Andy Pan <i@andypan.me>
Comment thread src/config.h Outdated
@codecov

codecov Bot commented Apr 25, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (unstable@e64d91c). Click here to learn what that means.

❗ Current head 5e918dd differs from pull request most recent head ac01cc4. Consider uploading reports for the commit ac01cc4 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##             unstable     #294   +/-   ##
===========================================
  Coverage            ?   68.41%           
===========================================
  Files               ?      108           
  Lines               ?    61562           
  Branches            ?        0           
===========================================
  Hits                ?    42117           
  Misses              ?    19445           
  Partials            ?        0           

Signed-off-by: Andy Pan <i@andypan.me>

@zuiderkwast zuiderkwast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

Can you update the PR description to mention the checks that couldn't have worked before without sys/params.h? I mean for which systems we never used accept4 but we do after this PR.

Just curios: Is MacOS included in any of these BSD checks?

@panjf2000

Copy link
Copy Markdown
Contributor Author

Can you update the PR description to mention the checks that couldn't have worked before without sys/params.h? I mean for which systems we never used accept4 but we do after this PR.

Done.

Just curios: Is MacOS included in any of these BSD checks?

No, macOS doesn't have system call accept4().

@zuiderkwast zuiderkwast merged commit 4948d53 into valkey-io:unstable Apr 25, 2024
@panjf2000 panjf2000 deleted the os-macros branch April 25, 2024 14:22
@zuiderkwast

Copy link
Copy Markdown
Contributor

Thanks! First I failed to see the point of the changes (I saw just adding defined() which didn't seem to matter) but it turned out to be much more.

WM0323 pushed a commit to WM0323/valkey that referenced this pull request Apr 26, 2024
…#294)

This PR has mainly done three things:

1. Enable `accept4()` on DragonFlyBSD
2. Fix the failures of determining the presence of `accept4()` due to
   the missing <sys/param.h> on two OSs: NetBSD, OpenBSD
3. Drop the support of FreeBSD < 10.0 for `valkey`

### References
- [param.h in
DragonFlyBSD](https://github.com/DragonFlyBSD/DragonFlyBSD/blob/7485684fa5c3fadb6c7a1da0d8bb6ea5da4e0f2f/sys/sys/param.h#L129-L257)
- [param.h in
FreeBSD](https://github.com/freebsd/freebsd-src/blob/main/sys/sys/param.h#L46-L76)
- [param.h in
NetBSD](https://github.com/NetBSD/src/blob/b5f8d2f930b7ef226d4dc1b4f7017e998c0e5cde/sys/sys/param.h#L53-L70)
- [param.h in
OpenBSD](https://github.com/openbsd/src/blob/d9c286e032a7cee9baaecdd54eb0d43f658ae696/sys/sys/param.h#L40-L45)

---------

Signed-off-by: Andy Pan <i@andypan.me>

Signed-off-by: Sher Sun <sher.sun@huawei.com>
@zuiderkwast zuiderkwast added the release-notes This issue should get a line item in the release notes label Apr 30, 2024
@panjf2000 panjf2000 mentioned this pull request May 9, 2024
zuiderkwast pushed a commit that referenced this pull request May 13, 2026
Squashed 'deps/libvalkey/' changes from 45c2ed15c..cb5ff91aa

cb5ff91aa Release 0.5.0
cd3d5d8ff Fix potential heap-buffer-overflow in cluster error reply parsing (#305)
32c7c5d09 Refactor error handling to use valkeySetErrorFromErrno and valkeyClearError (#303)
e4b548b32 Remove support for external dict while still supporting external sds (#302)
61b27c453 Try all addresses from DNS before failing to connect (#300)
8aff94fab RDMA: Fix lost EPOLLIN/POLLIN events
b9285b5fc Optimize read buffer compaction and reduce copying (#294)
0fa809877 Additional overflow protection for MAP/ATTR
b26c56e87 Fix gcc warnings (#287)

git-subtree-dir: deps/libvalkey
git-subtree-split: cb5ff91aa5816807f0a549bd2b36e611253dfeb6

Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
lucasyonge pushed a commit that referenced this pull request May 14, 2026
Squashed 'deps/libvalkey/' changes from 45c2ed15c..cb5ff91aa

cb5ff91aa Release 0.5.0
cd3d5d8ff Fix potential heap-buffer-overflow in cluster error reply parsing (#305)
32c7c5d09 Refactor error handling to use valkeySetErrorFromErrno and valkeyClearError (#303)
e4b548b32 Remove support for external dict while still supporting external sds (#302)
61b27c453 Try all addresses from DNS before failing to connect (#300)
8aff94fab RDMA: Fix lost EPOLLIN/POLLIN events
b9285b5fc Optimize read buffer compaction and reduce copying (#294)
0fa809877 Additional overflow protection for MAP/ATTR
b26c56e87 Fix gcc warnings (#287)

git-subtree-dir: deps/libvalkey
git-subtree-split: cb5ff91aa5816807f0a549bd2b36e611253dfeb6

Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes This issue should get a line item in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants