Attention is currently required from: Dmitry Zhadinets, Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/86875?usp=email )
Change subject: libflashrom: Update the API for Logger Callback ......................................................................
Patch Set 7:
(2 comments)
File libflashrom.c:
https://review.coreboot.org/c/flashrom/+/86875/comment/13485125_4d671307?usp... : PS6, Line 84: if (!log_callback && global_log_callback == format_message_and_invoke_log_callback_v2) : global_log_callback = NULL;
*The logic of this condition was to reset log callback v**1** only in the case if it was installed b […]
I wasn't realising at the time of writing the comment that I found a bug :) But I am so happy that I could help.
Thank you for detailed explanation of use case you had in mind, now I understand why you added a test which was mixing v1 and v2 together in one scenario.
Sorry that I asked you to remove the test! Now that I understand, you can add it back - if you want of course. I would still want to keep two granular tests (the ones which are in current patchset already), also no global state. But if you still interested, you can add back the test which sets/resets callbacks using combination of v1 and v2. As you decide.
File libflashrom.c:
https://review.coreboot.org/c/flashrom/+/86875/comment/87154054_de01e76c?usp... : PS7, Line 75: if (((unsigned)size+1) > sizeof(message)) : return -ERANGE; A question here: I remember initial comment thread was about fallback to dymanic allocation if message size is above limit. But this code returns an error if the message is above limit, and does not print the message?
-
Anastasia Klimchuk (Code Review)