Change in flashrom[master]: print.c: Fix pp str format alignment in print_supported_boards_helper()
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37372 )
Change subject: print.c: Fix pp str format alignment in print_supported_boards_helper() ......................................................................
print.c: Fix pp str format alignment in print_supported_boards_helper()
Fix a print format regression introduced in commit 61e16e549a52194ac80ef40504f2dc661d1ff99c.
Change-Id: Ic25512bc6f31e62dfc77e32a4c71519bdde01598 Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M print.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/37372/1
diff --git a/print.c b/print.c index 596fc53..aeb56cb 100644 --- a/print.c +++ b/print.c @@ -401,7 +401,7 @@ for (i = 0; i < maxboardlen - strlen(b->name); i++) msg_ginfo(" ");
- msg_pinfo(test_state_to_text(b->working)); + msg_pinfo("%s\t", test_state_to_text(b->working));
for (e = board_matches; e->vendor_name != NULL; e++) { if (strcmp(e->vendor_name, b->vendor)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37372 )
Change subject: print.c: Fix pp str format alignment in print_supported_boards_helper() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/flashrom/+/37372/1/print.c File print.c:
https://review.coreboot.org/c/flashrom/+/37372/1/print.c@404 PS1, Line 404: msg_pinfo wwhy isn't this `msg_ginfo` like everything else?
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37372 )
Change subject: print.c: Fix pp str format alignment in print_supported_boards_helper() ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
I like the idea of aligning to tabs, but I think that will require a lot more effort to fix the output. I tested an alternative approach (linked in the comment, along with a longer explanation).
https://review.coreboot.org/c/flashrom/+/37372/1/print.c File print.c:
https://review.coreboot.org/c/flashrom/+/37372/1/print.c@404 PS1, Line 404: msg_pinfo
wwhy isn't this `msg_ginfo` like everything else?
Unfortunately this still shows some unaligned entries in `flashrom -L` output. The problem is that the strings that test_state_to_text() outputs are significantly longer than the strings that were output originally, and some are longer than a tabstop. (And tabstop is defined by the user's terminal, so that's another can of worms...)
Another issue is that the preceding strings aren't necessarily aligned on tabs. Instead, they use for-loops to pad the field with spaces up to its maximum length.
With all this in mind, I came up with an alternative approach that seems to fix the issue: https://review.coreboot.org/c/flashrom/+/37407
Edward O'Callaghan has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/37372 )
Change subject: print.c: Fix pp str format alignment in print_supported_boards_helper() ......................................................................
Abandoned
Incomplete solution.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37372 )
Change subject: print.c: Fix pp str format alignment in print_supported_boards_helper() ......................................................................
Patch Set 1:
Patch Set 1: Code-Review-1
(1 comment)
I like the idea of aligning to tabs, but I think that will require a lot more effort to fix the output. I tested an alternative approach (linked in the comment, along with a longer explanation).
Regardless thanks for the quick review! I looked over your patch and seems good to me.
-
Angel Pons (Code Review) -
David Hendricks (Code Review)
-
Edward O'Callaghan (Code Review)