Skip to content

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Jun 24, 2024

The AC_CHECK_HEADERS macro defines HAVE_SYS_SYSCTL_H if the sys/sysctl.h header is found. However, in the source code, this header is guarded by HAVE_SYSCTL and HAVE_SYSCTL_ARND macros, which have their own checks. Since HAVE_SYS_SYSCTL_H is not used, we can skip the AC_CHECK_HEADERS(... sys/sysctl.h ...) check.

The `AC_CHECK_HEADERS` macro defines `HAVE_SYS_SYSCTL_H` if the
`sys/sysctl.h` header is found. However, in the source code, this header
is guarded by `HAVE_SYSCTL` and `HAVE_SYSCTL_ARND` macros, which have
their own checks. Since `HAVE_SYS_SYSCTL_H` is not used, we can skip the
`AC_CHECK_HEADERS(... sys/sysctl.h ...)` check.
@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 24, 2024

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK laanwj, fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@laanwj
Copy link
Member

laanwj commented Jun 26, 2024

ACK c0b5ea5
verified there is no HAVE_SYS_SYSCTL_H in the code, nor ever was, while it was introduced at the same time as HAVE_SYSCTL

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK c0b5ea5 - we could got the other way, and add nested #defs, but that doesn't seem worthwhile.

@fanquake fanquake merged commit d3d2bbf into bitcoin:master Jun 26, 2024
@hebasto hebasto deleted the 240624-sysctl branch June 26, 2024 14:26
@hebasto
Copy link
Member Author

hebasto commented Jul 14, 2024

There is nothing to port to the CMake staging branch. Deleting the "Needs CMake port" label.

PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 22, 2025
c0b5ea5 build: Drop redundant `sys/sysctl.h` header check (Hennadii Stepanov)

Pull request description:

  The `AC_CHECK_HEADERS` macro defines `HAVE_SYS_SYSCTL_H` if the `sys/sysctl.h` header is found. However, in the source code, this header is guarded by `HAVE_SYSCTL` and `HAVE_SYSCTL_ARND` macros, which have their own checks. Since `HAVE_SYS_SYSCTL_H` is not used, we can skip the `AC_CHECK_HEADERS(... sys/sysctl.h ...)` check.

ACKs for top commit:
  laanwj:
    ACK c0b5ea5
  fanquake:
    ACK c0b5ea5 - we could got the other way, and add nested #defs, but that doesn't seem worthwhile.

Tree-SHA512: 73bc4bbfc5c457cd2c38e40f8e57d2a70c06ef661d76d4148d683d262be45b9405b8cda1958ac611c312ca7d9e2f9624cf2cac1b61f1008af0856875c62f0eac
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 22, 2025
c0b5ea5 build: Drop redundant `sys/sysctl.h` header check (Hennadii Stepanov)

Pull request description:

  The `AC_CHECK_HEADERS` macro defines `HAVE_SYS_SYSCTL_H` if the `sys/sysctl.h` header is found. However, in the source code, this header is guarded by `HAVE_SYSCTL` and `HAVE_SYSCTL_ARND` macros, which have their own checks. Since `HAVE_SYS_SYSCTL_H` is not used, we can skip the `AC_CHECK_HEADERS(... sys/sysctl.h ...)` check.

ACKs for top commit:
  laanwj:
    ACK c0b5ea5
  fanquake:
    ACK c0b5ea5 - we could got the other way, and add nested #defs, but that doesn't seem worthwhile.

Tree-SHA512: 73bc4bbfc5c457cd2c38e40f8e57d2a70c06ef661d76d4148d683d262be45b9405b8cda1958ac611c312ca7d9e2f9624cf2cac1b61f1008af0856875c62f0eac
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Apr 23, 2025
1e04c56 Merge bitcoin#30556: doc: multisig-tutorial: remove obsolete mention and link to closed PR (merge-script)
fd43510 Merge bitcoin#30453: test: Non-Shy version sender (glozow)
e3c3a11 Merge bitcoin#30327: build: Drop redundant `sys/sysctl.h` header check (merge-script)
808a77d Merge bitcoin#30156: fuzz: More accurate coverage reports (merge-script)
3ca42ba Merge bitcoin#28874: doc: fixup help output for -upnp and -natpmp (merge-script)
ea32090 Merge bitcoin#28461: build: Windows SSP roundup (fanquake)
e71c422 Merge bitcoin#28151: build: use `-muse-unaligned-vector-move` for Windows builds (fanquake)
077bbb4 Merge bitcoin#28131: test: Add UBSan `-fsanitize=integer` suppressions for `src/secp256k1` subtree (fanquake)
de5a2d1 Merge bitcoin#27940: test: Add implicit-signed-integer-truncation:*/include/c++/ suppression (fanquake)

Pull request description:

  ## Issue being fixed or feature implemented
  Batch of trivial backports

  ## How Has This Been Tested?
  Built locally

  ## Breaking Changes

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 1e04c56
  knst:
    utACK 1e04c56

Tree-SHA512: 5e9a3fc4ac2ea06e8da48952bbdb43e7ed0c3d9ab3fdae3d8753bbe10b957c6cbb06e01b9860db4cd5ade91c8cd419dbbc8ee76073d00b4d6ff0f6ae6a4cbfd2
@bitcoin bitcoin locked and limited conversation to collaborators Jul 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants