util: Don’t reference errno when pthread fails. #19194

1. 
   miztake commented at 5:56 pm on June 6, 2020: contributor

   Pthread library does not set errno. Pthread library’s errno is returned by return value.
2. DrahtBot added the label Utils/log/libs on Jun 6, 2020
3. 
   practicalswift commented at 7:31 pm on June 6, 2020: contributor

   Concept ACK: nice catch and nice first-time contribution!

   May I ask how did you found this issue? :)

   Warm welcome as a contributor @miztake :)

4. 
   pstratem commented at 0:21 am on June 7, 2020: contributor

   ACK c7601aba89972f71464c9809cc3607b96b5975ab

   you can move the declaration into the if statement like this: if ((int rc = pthread_setschedparam(pthread_self(), SCHED_BATCH, &param)) != 0) {

5. in src/util/system.cpp:1167 in c7601aba89 outdated

   1162@@ -1163,8 +1163,9 @@ void ScheduleBatchPriority()
   1163 {
   1164 #ifdef SCHED_BATCH
   1165     const static sched_param param{};
   1166-    if (pthread_setschedparam(pthread_self(), SCHED_BATCH, &param) != 0) {
   1167-        LogPrintf("Failed to pthread_setschedparam: %s\n", strerror(errno));
   1168+    int rc;
   1169+    if ((rc = pthread_setschedparam(pthread_self(), SCHED_BATCH, &param)) != 0) {
   

   ---

   ---

   

   hebasto commented at 10:44 am on June 7, 2020:

   nit: could look clearer

   0    const int rc = pthread_setschedparam(pthread_self(), SCHED_BATCH, &param);
   1    if (rc != 0) {
   

   … until switching to C++17 :)

   ---

   

   miztake commented at 3:26 pm on June 7, 2020:

   Hi, @hebasto

   Thank you for suggesting change. I accept your suggested change.

6. 
   hebasto commented at 10:47 am on June 7, 2020: member

   ACK c7601aba89972f71464c9809cc3607b96b5975ab, tested on Linux Mint 19.3 (x86_64).

   From the pthread_setschedparam() docs:

   On success, these functions return 0; on error, they return a nonzero error number.

7. 
   hebasto commented at 10:50 am on June 7, 2020: member

   you can move the declaration into the if statement like this: if ((int rc = pthread_setschedparam(pthread_self(), SCHED_BATCH, &param)) != 0) {

   It won’t compile :) (at least on GCC 7.5).

8. 
   miztake commented at 3:26 pm on June 7, 2020: contributor

   Hi, @practicalswift

   May I ask how did you found this issue? :)

   Thank you for your comment. I didn’t actually output this log. It’s a common bug when using pthread and I found it in a code review. ( just greped “pthread” :) )

9. 
   donaloconnor commented at 3:49 pm on June 7, 2020: contributor

   ACK 0e7b64b64db00d03489a72c988dc7ef53433f69d
10. hebasto approved
11. 
    hebasto commented at 3:52 pm on June 7, 2020: member

    ACK 0e7b64b64db00d03489a72c988dc7ef53433f69d.

    Mind squashing commits into one commit?

12. 
    kristapsk commented at 4:29 pm on June 7, 2020: contributor

    Concept ACK, checked man pthread_setschedparam, change seems correct, but commits should be squashed into one.
13. util: Don't reference errno when pthread fails.

    Pthread library does not set errno.
    Pthread library's errno is returned by return value.
    
    Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>

    cb38b069b0
14. 
    miztake commented at 7:42 am on June 8, 2020: contributor

    Hi, @kristapsk

    Thank you for your comment. I squashed the commits into one.

15. hebasto approved
16. 
    hebasto commented at 7:46 am on June 8, 2020: member

    ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08, only squashed commits since the previous review.
17. 
    MarcoFalke commented at 10:41 am on June 8, 2020: member

    review ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08
18. 
    practicalswift commented at 11:13 am on June 8, 2020: contributor

    ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08 – patch looks correct
19. fanquake merged this on Jun 8, 2020
20. fanquake closed this on Jun 8, 2020

    ---

21. fanquake added the label Needs backport (0.20) on Jun 8, 2020
22. 
    laanwj commented at 11:58 am on June 9, 2020: member

    Poshumous ACK, good catch!
23. fanquake referenced this in commit 0596a6eeb5 on Jun 9, 2020
24. fanquake removed the label Needs backport (0.20) on Jun 9, 2020
25. fanquake referenced this in commit 01c563708f on Jul 10, 2020
26. backpacker69 referenced this in commit 278f2eeffe on Sep 8, 2020
27. fanquake referenced this in commit a825377c8a on Oct 14, 2020
28. fanquake referenced this in commit fa5f91a4ec on Oct 15, 2020
29. fanquake referenced this in commit 27bb2cc3b6 on Oct 16, 2020
30. Bushstar referenced this in commit 77ef85cf89 on Oct 21, 2020
31. Platinumwrist referenced this in commit 05e2740d92 on Oct 25, 2020
32. MarcoFalke referenced this in commit 5baaea4410 on Dec 2, 2020
33. Fabcien referenced this in commit 1fbbcdcdaa on May 14, 2021
34. DrahtBot locked this on Feb 15, 2022

Contributors
miztake practicalswift pstratem hebasto donaloconnor kristapsk MarcoFalke laanwj

Labels
Utils/log/libs