trivial: Replace CPubKey::operator[] with CPubKey::vch where possible #13722

pull hmel wants to merge 1 commits into bitcoin:master from hmel:use_varname_not_this changing 1 files +4 −4

hmel commented at 6:52 AM on July 20, 2018: contributor

Use variable name instead of calling operator[] through &(*this)[0]
MarcoFalke added the label Refactoring on Jul 20, 2018
promag commented at 1:49 PM on July 20, 2018: member

Actually it's not dependent of class layout, it uses operator[]: https://github.com/bitcoin/bitcoin/blob/c575260f80e94026204f97714ba6bcb26237d818/src/pubkey.h#L113 So I suggest to update the PR and commit.

Having said that, I like your change, but don't know if there is a motivation for the current code. Maybe @sipa has something to say?
hmel commented at 3:04 PM on July 20, 2018: contributor

You're absolutely right. I overlooked operator[], but I still think using variable name directly instead of &(*this)[0] going through operator[] to get the same thing. Maybe I'm missing something else? @sipa
sipa commented at 4:12 PM on July 20, 2018: member

Indeed, it is not dependent on class layout (no cast to char pointer is made), but the new code certainly looks better.
promag commented at 4:15 PM on July 20, 2018: member

@hmel Please update PR and commit messages.

ACK the change.
hmel renamed this:
~~[Trivial] Use variable name instead of relying on class layout~~
[Trivial] Use variable name instead of calling operator[] through *this
on Jul 20, 2018
sipa commented at 6:01 PM on July 20, 2018: member

utACK b0bb82209baceff2ae8ff566c374d1dd7725765e
promag commented at 6:03 PM on July 20, 2018: member

Commit message still wrong.
hmel commented at 1:18 AM on July 21, 2018: contributor

Care to make a suggestion for commit message?
promag commented at 1:42 AM on July 21, 2018: member

Sure, currently it's Use variable name instead of relying on class layout, how about trivial: Replace CPubKey::operator[] with CPubKey::vch where possible?
trivial: Replace CPubKey::operator[] with CPubKey::vch where possible 6755569840
fanquake renamed this:
~~[Trivial] Use variable name instead of calling operator[] through *this~~
trivial: Replace CPubKey::operator[] with CPubKey::vch where possible
on Jul 21, 2018
hmel force-pushed on Jul 21, 2018
l2a5b1 commented at 3:01 PM on July 21, 2018: contributor

Nice improvement, utACK 6755569
promag commented at 3:04 PM on July 21, 2018: member

utACK 6755569.
fanquake commented at 3:28 AM on July 22, 2018: member

utACK 6755569
fanquake added this to the "Mergeable" column in a project

MarcoFalke merged this on Jul 22, 2018
MarcoFalke closed this on Jul 22, 2018
MarcoFalke referenced this in commit 59407fce54 on Jul 22, 2018
laanwj commented at 1:30 PM on July 22, 2018: member

Post=merge ACK, nice cleanup
hmel deleted the branch on Jul 22, 2018
fanquake removed this from the "Mergeable" column in a project

jasonbcox referenced this in commit c24fca0bda on Oct 25, 2019
jonspock referenced this in commit 5bd735688f on Dec 27, 2019
jonspock referenced this in commit 525c142a95 on Dec 29, 2019
PastaPastaPasta referenced this in commit 6742fad5c6 on Jul 29, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels