handler() variable redundancy/naming cleanup #35
Conversation
davidklaftenegger
force-pushed
the
variable_cleanup
branch
from
206fc7e
to
9950e7b
Compare
| char* aligned_access_ptr = reinterpret_cast<char*>(startAddr) + aligned_access_offset; | ||
| unsigned long startIndex = getCacheIndex(aligned_access_offset); | ||
| unsigned long homenode = getHomenode(aligned_access_offset); | ||
| unsigned long offset = getOffset(aligned_access_offset); | ||
| unsigned long id = 1 << getID(); | ||
| unsigned long invid = ~id; |
There was a problem hiding this comment.
Eventually we should ensure that all variables have better descriptive and more consistent names, in addition to using snake_case in accordance with the code standard. Not important for this specific PR, but the simple fact that id equals 1 << getID() rather than simply the return value of getID() is not straightforward.
There was a problem hiding this comment.
Sure, eventually I would like to get rid of everything in this file and replace it with more modular, easier to read, better documented, and more consistent code. Should we create another ticket for this particular complaint, in the hope someone addresses it?
There was a problem hiding this comment.
I believe it is out of the scope of this PR, as changing either of the variable names above would lead to changes on multiple locations in the backend. Sounds like a good idea for another ticket to be dealt with in the future.
There was a problem hiding this comment.
This is now issue #36, please feel free to contribute your find there.
davidklaftenegger
dismissed stale reviews from pekemark, lundgren87, and SakalisC
via
07f6d8c
davidklaftenegger
force-pushed
the
variable_cleanup
branch
from
9950e7b
to
07f6d8c
Compare
This patch cleans up some redundant variables and confusing naming in the segfault handler function.
Due to my inability to use github nicely, this replaces #34