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

fix macro HAVE_O_CLOEXEC when O_CLOEXEC not found #1028

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

emptyx0
Copy link

@emptyx0 emptyx0 commented Jun 9, 2022

port_config.h define HAVE_O_CLOEXEC is 0 when O_CLOEXEC not found in fcntl.h, so condition '#if defined(HAVE_O_CLOEXEC)' hit. It could be verified on centos5

@google-cla
Copy link

google-cla bot commented Jun 9, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@emptyx0
Copy link
Author

emptyx0 commented Jun 25, 2022

macro HAVE_O_CLOEXEC is defined automatic by cmake in file port_config.h.in, cmake will define HAVE_O_CLOEXEC equals 0, so #if directive on HAVE_O_CLOEXEC in file env_posix.cc should be '#if HAVE_O_CLOEXEC' , not '#if defined(HAVE_O_CLOEXEC)',

@maflcko
Copy link
Contributor

maflcko commented Jun 29, 2022

@emptyx0 Did you want to sign the CLA?

@emptyx0
Copy link
Author

emptyx0 commented Jun 30, 2022

@emptyx0 Did you want to sign the CLA?

I have already signed the CLA , 3Q dalao

fanquake added a commit to bitcoin-core/leveldb-subtree that referenced this pull request Jul 27, 2022
1eeb1cb fix macro HAVE_O_CLOEXEC when O_CLOEXEC not found (emptyx.wong)

Pull request description:

  google#624 was [buggy](google#624 (comment)).

  One bug was fixed in google@27dc99f.

  google#1028 addresses another one.

  This PR cherry-picks from google#1028, and is an alternative to bitcoin/bitcoin#25463.

ACKs for top commit:
  fanquake:
    ACK 1eeb1cb - I think the likelyhood of this being merged upstream is quite low, so that shouldn't be a blocker in us merging it here.
  jarolrod:
    ACK 1eeb1cb

Tree-SHA512: 67a4e05372e71ba12abf84c633284afbaa54852b616fc86939231e72d5054d9f7973caad5a18944a84f0fdf924f404a35b50287d53a704c7d320f04068800ebf
@hebasto
Copy link

hebasto commented Nov 5, 2023

Also see #1149.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants