util: make ScheduleBatchPriority advisory only #17604

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:fix_ScheduleBatchPriority_comment changing 2 files +3 −9

fanquake commented at 10:36 PM on November 25, 2019: member

No description provided.
fanquake added the label Docs on Nov 25, 2019
in src/util/system.h:371 in 30e909aa42 outdated
```
 366 | @@ -367,8 +367,8 @@ std::string CopyrightHolders(const std::string& strPrefix);
 367 |   * On platforms that support it, tell the kernel the calling thread is
 368 |   * CPU-intensive and non-interactive. See SCHED_BATCH in sched(7) for details.
 369 |   *
 370 | - * @return The return value of sched_setschedule(), or 1 on systems without
 371 | - * sched_setschedule().
 372 | + * @return The return value of pthread_setschedparam(), or 1 on systems without
 373 | + * SCHED_BATCH.
```
laanwj commented at 1:15 PM on November 26, 2019:

I don't think it's necessary to describe implementation details here at all. Maybe just state it returns 0 on success and any other value on failure or when the functionality is not supported on the OS.

Note that we don't use the return value at all, not even for logging. This also differs from our normal convention of bool for success/failure.

I think I'd prefer making this a purely advisory function, that returns void.

fanquake commented at 3:27 PM on November 26, 2019:

I think I'd prefer making this a purely advisory function, that returns void.

I've updated the PR to do that.
util: make ScheduleBatchPriority advisory only d2a3a5cadb
fanquake force-pushed on Nov 26, 2019
fanquake renamed this:
~~doc: correct ScheduleBatchPriority return value documentation~~
util: make ScheduleBatchPriority advisory only
on Nov 26, 2019
laanwj commented at 4:13 PM on November 26, 2019: member

ACK d2a3a5cadbe58c0fe363bbc6acac293d41eedf7e
fanquake removed the label Docs on Nov 26, 2019
fanquake added the label Utils/log/libs on Nov 26, 2019
laanwj referenced this in commit 23cecd6cd5 on Nov 28, 2019
laanwj merged this on Nov 28, 2019
laanwj closed this on Nov 28, 2019
practicalswift commented at 11:57 AM on November 28, 2019: contributor

@fanquake The description for this PR is empty: can you describe why this change is needed? :)
laanwj commented at 12:02 PM on November 28, 2019: member

It removes an unused return value, where a weird return convention was used. Instead of documenting it, it was better to remove the return value completely.
sidhujag referenced this in commit bef547ea64 on Nov 28, 2019
MarkLTZ referenced this in commit ec87ba9339 on Nov 29, 2019
fanquake deleted the branch on Jan 4, 2020
deadalnix referenced this in commit 98451376c4 on Apr 27, 2020
ftrader referenced this in commit 8908015742 on Aug 17, 2020
sidhujag referenced this in commit 20b400a08c on Nov 10, 2020
DrahtBot locked this on Feb 15, 2022

Contributors

Labels