fuzz: Use ConsumeWeakEnum in addrman for service flags #21487

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2103-fuzzEnumWeak changing 1 files +1 −1

MarcoFalke commented at 11:05 AM on March 20, 2021: member

This has minimally better performance. Reported by me in #20228 (review)
fuzz: Use ConsumeWeakEnum in addrman for service flags 55554463c1
fanquake added the label Tests on Mar 20, 2021
MarcoFalke commented at 11:07 AM on March 20, 2021: member

I am also thinking about renaming this to ConsumeEnumFlags to better reflect what it does, but maybe this can be done later or the existing name is fine already?
practicalswift commented at 5:32 PM on March 20, 2021: contributor

Concept ACK on both using ConsumeWeakEnum and renaming it ConsumeEnumFlags.

Better naming will be helpful for future generations of Bitcoin Core fuzzing enthusiasts, and ConsumeEnumFlags sure is a better name :)
practicalswift commented at 10:42 AM on March 21, 2021: contributor

Tested ACK 55554463c15e3c75a64d26209dd3f8396e508cc2
vasild approved
vasild commented at 1:03 PM on March 21, 2021: member

ACK 55554463c15e3c75a64d26209dd3f8396e508cc2

I think the existent name "...weak...()" is good because the function may return a completely arbitrary value that is not in the enum. ConsumeEnumFlags() may give the wrong impression that it only returns enumerated values.
MarcoFalke commented at 5:49 PM on March 21, 2021: member

Though, "weak" could also be interpreted as "weakly typed enum", meaning a non-C++11-"strongly typed enum". But this interpretation would be wrong because flags can also be implemented with strongly typed enums. Maybe there is a better name that we haven't yet thought of?
MarcoFalke merged this on Mar 23, 2021
MarcoFalke closed this on Mar 23, 2021
MarcoFalke deleted the branch on Mar 23, 2021
MarcoFalke commented at 8:47 AM on March 23, 2021: member

On really verbose name could be ConsumeUnderlyingTypeEnum?

MarcoFalke commented at 1:41 PM on March 23, 2021: member

[09:35] <vasild> MarcoFalke: maybe ConsumeEnumOrRandom() better describes it?

sidhujag referenced this in commit 63198f2302 on Mar 23, 2021
DrahtBot locked this on Aug 16, 2022

Contributors

Labels

Linked (view graph)

#20228 addrman: Make addrman a top-level component