The last field of a register was not printed #908
Conversation
`riscv_debug_reg_fields_to_s()` exited early without processing the last field. Signed-off-by: Evgeniy Naydanov <[email protected]>
| for (struct riscv_debug_reg_field_list_t list; get_next; get_next = list.get_next) { | ||
| list = get_next(context); |
There was a problem hiding this comment.
I see how the old code would omit the last field, because when list.get_next is NULL you still need to print out one more field.
| struct riscv_debug_reg_field_list_t (*get_next)(riscv_debug_reg_ctx_t contex), | ||
| riscv_debug_reg_ctx_t context, uint64_t value) |
There was a problem hiding this comment.
Why do you need to pass a function get_next instead of the list?
Also, I'm surprised that there are no callers to this function that needed to be updated.
There was a problem hiding this comment.
Why do you need to pass a function get_next instead of the list?
IMHO it makes the loop more concise. If one were to transition to passing a list node, the loop would be required to be a do ... while () since there would always be at least one node. I find for loops easier to read. Don't you agree?
Also, I'm surprised that there are no callers to this function that needed to be updated.
This is a static function, only called from riscv_debug_reg_to_s(). The appropriate adjustment (transition from passing a list node to passing a function pointer which generates such node) is made on line 90.
There was a problem hiding this comment.
I missed the github info showing me that line 90 was a different function. Thanks for explaining. This is fine.
| struct riscv_debug_reg_field_list_t (*get_next)(riscv_debug_reg_ctx_t contex), | ||
| riscv_debug_reg_ctx_t context, uint64_t value) |
There was a problem hiding this comment.
I missed the github info showing me that line 90 was a different function. Thanks for explaining. This is fine.
riscv_debug_reg_fields_to_s()exited early without processing the last field.