Skip to content

Conversation

@Empact
Copy link
Contributor

@Empact Empact commented Jul 9, 2018

At least SmartOS implements FD_ZERO relative to memset but does not
ensure its availability. Including cstring fixes that.

@Empact
Copy link
Contributor Author

Empact commented Jul 9, 2018

Should fix #13581. @stacepellegrino are you up for building this branch?

At least SmartOS implements FD_ZERO relative to memset but does not
ensure its availability. Including cstring fixes that.
@Empact Empact changed the title Inlcude cstring rather than cstddef in compat/glib_sanity.cpp Include cstring alongside select in compat/glib_sanity.cpp Jul 9, 2018
@Empact
Copy link
Contributor Author

Empact commented Jul 9, 2018

Alternatively we could include cstring all the time and drop the extern definition for memcpy.
/cc @theuni 679240d

@Empact
Copy link
Contributor Author

Empact commented Jul 10, 2018

Closing this since I'm doubting #13581 now.

@Empact Empact closed this Jul 10, 2018
@Empact Empact reopened this Jul 11, 2018
@theuni
Copy link
Member

theuni commented Jul 11, 2018

Yes, this is not correct. We're attempting to replace memcpy, so including cstring is not the right thing to do. I'll comment further on #13581.

@Empact
Copy link
Contributor Author

Empact commented Jul 13, 2018

Closing as per #13581 (comment)

@Empact Empact closed this Jul 13, 2018
@Empact Empact deleted the glib-sanity branch July 13, 2018 18:15
laanwj added a commit that referenced this pull request Sep 18, 2019
b4fd0ca Include cstring for sanity_test_fdelt if required (Ben Woosley)
7fb886b [moveonly] Split glibc sanity_test_fdelt out (Ben Woosley)

Pull request description:

  SmartOS FD_ZERO is implemented in a way that requires
  an external declaration of memcpy. We can not simply
  include cstring in the existing file because
  sanity_test_memcpy is attempting to replace memcpy.

  Instead split glibc_sanity into fdelt and memcpy files,
  and include <cstring> in glibc_sanity/fdelt.cpp.

  Fixes #13581, see also #13619

ACKs for top commit:
  laanwj:
    Code review an lightly tested (but not on SmartOS) ACK b4fd0ca

Tree-SHA512: 231306da291ad9eca8ba91bea1e9c27b6c2e96e484d1602e1c2cf27761202f9287ce0bc19fefd000943d2b449d0e5929cd39e2f7e09cf930d89fa520228ccbec
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 23, 2019
b4fd0ca Include cstring for sanity_test_fdelt if required (Ben Woosley)
7fb886b [moveonly] Split glibc sanity_test_fdelt out (Ben Woosley)

Pull request description:

  SmartOS FD_ZERO is implemented in a way that requires
  an external declaration of memcpy. We can not simply
  include cstring in the existing file because
  sanity_test_memcpy is attempting to replace memcpy.

  Instead split glibc_sanity into fdelt and memcpy files,
  and include <cstring> in glibc_sanity/fdelt.cpp.

  Fixes bitcoin#13581, see also bitcoin#13619

ACKs for top commit:
  laanwj:
    Code review an lightly tested (but not on SmartOS) ACK b4fd0ca

Tree-SHA512: 231306da291ad9eca8ba91bea1e9c27b6c2e96e484d1602e1c2cf27761202f9287ce0bc19fefd000943d2b449d0e5929cd39e2f7e09cf930d89fa520228ccbec
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants