Skip to content

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented Apr 9, 2018

Nowhere in the man page of pthread_setschedparam it is mentioned that 0 is a valid value. The example uses pthread_self(), so should we.

(noticed by Anthony Towns)
Fixes #12915.

Nowhere in the man page of `pthread_setschedparam` it is mentioned that
`0` is a valid value. The example uses `pthread_self()`, so should we.

(noticed by Anthony Towns)
@maflcko
Copy link
Member

maflcko commented Apr 9, 2018

@eklitzke Mind to take a look here?

@maflcko
Copy link
Member

maflcko commented Apr 9, 2018

Concept ACK cff66e6 (fixes the issue for me, didn't review though)

@laanwj laanwj requested a review from eklitzke April 9, 2018 13:54
@practicalswift
Copy link
Contributor

utACK cff66e6

@laanwj
Copy link
Member Author

laanwj commented Apr 9, 2018

So on IRC it was mentioned that

<aj> hmm, i thought designator initialisers (param{.sched_priority=0}) weren't ok for c++11 (http://en.cppreference.com/w/cpp/language/aggregate_initialization says they're c++20 fwiw)

Should I make this change here too? It's curious that it does compile (as our build explicitly passes -std=c++11), but if it's not allowed by the standard we should probably change it.

@ajtowns
Copy link
Contributor

ajtowns commented Apr 9, 2018

utACK cff66e6

Neither clang nor gcc seems to warn about designated initializers with c++11. I think this is the only use of one in the codebase currently, so removing it is probably fine.

Although no compiler appears to complain about it, these are
not valid for c++11.
(http://en.cppreference.com/w/cpp/language/aggregate_initialization says they're c++20)

The structure is defined as:

   struct sched_param {
       int sched_priority;
   };

So passing 0 for the first field has the same effect.
@promag
Copy link
Contributor

promag commented Apr 9, 2018

utACK cff66e6.

Should I make this change here too?

Maybe just fix the problem for now.

Looks good.

@promag
Copy link
Contributor

promag commented Apr 9, 2018

utACK b86730a.

@ajtowns
Copy link
Contributor

ajtowns commented Apr 9, 2018

utACK b86730a

@laanwj laanwj merged commit b86730a into bitcoin:master Apr 9, 2018
laanwj added a commit that referenced this pull request Apr 9, 2018
…ead of 0

b86730a util: Remove designator initializer from ScheduleBatchPriority (Wladimir J. van der Laan)
cff66e6 util: Pass pthread_self() to pthread_setschedparam instead of 0 (Wladimir J. van der Laan)

Pull request description:

  Nowhere in the man page of `pthread_setschedparam` it is mentioned that `0` is a valid value. The example uses `pthread_self()`, so should we.

  (noticed by Anthony Towns)
  Fixes #12915.

Tree-SHA512: 249e93b1ae7e3ba28de6ee6288400b91d21ca1b4ca41d82211f6c9609b62deb5ac87182c7bf08471d3a3e0c1af314c9ecd41f8ae864febe963b1de8a816dd82f
@sidhujag
Copy link

I Noticed this aswell.. fixed it with self()

Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Aug 30, 2019
…ead of 0

Summary:
b86730a util: Remove designator initializer from ScheduleBatchPriority (Wladimir J. van der Laan)
cff66e6 util: Pass pthread_self() to pthread_setschedparam instead of 0 (Wladimir J. van der Laan)

Pull request description:

  Nowhere in the man page of `pthread_setschedparam` it is mentioned that `0` is a valid value. The example uses `pthread_self()`, so should we.

  (noticed by Anthony Towns)
  Fixes #12915.

Tree-SHA512: 249e93b1ae7e3ba28de6ee6288400b91d21ca1b4ca41d82211f6c9609b62deb5ac87182c7bf08471d3a3e0c1af314c9ecd41f8ae864febe963b1de8a816dd82f

Backport of Core PR12923
bitcoin/bitcoin#12923

Depends on D3953

Test Plan:
  make check
  test_runner.py

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: deadalnix, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3954
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 16, 2020
…am instead of 0

b86730a util: Remove designator initializer from ScheduleBatchPriority (Wladimir J. van der Laan)
cff66e6 util: Pass pthread_self() to pthread_setschedparam instead of 0 (Wladimir J. van der Laan)

Pull request description:

  Nowhere in the man page of `pthread_setschedparam` it is mentioned that `0` is a valid value. The example uses `pthread_self()`, so should we.

  (noticed by Anthony Towns)
  Fixes bitcoin#12915.

Tree-SHA512: 249e93b1ae7e3ba28de6ee6288400b91d21ca1b4ca41d82211f6c9609b62deb5ac87182c7bf08471d3a3e0c1af314c9ecd41f8ae864febe963b1de8a816dd82f
furszy added a commit to PIVX-Project/PIVX that referenced this pull request Mar 13, 2021
…ead of 0

48b3bc4 util: Remove designator initializer from ScheduleBatchPriority (Wladimir J. van der Laan)
886b6ca util: Pass pthread_self() to pthread_setschedparam instead of 0 (Wladimir J. van der Laan)

Pull request description:

  Coming straight from bitcoin#12923.

  Fixes a bug introduced in #2212 for **some** POSIX linux systems (observed on CentOS 7 currently)

  Original upstream text:

  > Nowhere in the man page of pthread_setschedparam it is mentioned that 0 is a valid value. The example uses pthread_self(), so should we.

ACKs for top commit:
  furszy:
    utACK 48b3bc4
  random-zebra:
    utACK 48b3bc4

Tree-SHA512: b7976efbe4659a20ca88e958dc419d6cf9a11867c0a1dc021af722f5878b4574a160680b35fecca348231c5477f52fd0a1e336f76968127acf0f5c018e2b6de7
TheArbitrator pushed a commit to TheArbitrator/dash that referenced this pull request Jun 4, 2021
…am instead of 0

b86730a util: Remove designator initializer from ScheduleBatchPriority (Wladimir J. van der Laan)
cff66e6 util: Pass pthread_self() to pthread_setschedparam instead of 0 (Wladimir J. van der Laan)

Pull request description:

  Nowhere in the man page of `pthread_setschedparam` it is mentioned that `0` is a valid value. The example uses `pthread_self()`, so should we.

  (noticed by Anthony Towns)
  Fixes bitcoin#12915.

Tree-SHA512: 249e93b1ae7e3ba28de6ee6288400b91d21ca1b4ca41d82211f6c9609b62deb5ac87182c7bf08471d3a3e0c1af314c9ecd41f8ae864febe963b1de8a816dd82f
@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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segmentation fault in util: ScheduleBatchPriority

7 participants