Attention is currently required from: Anastasia Klimchuk, Peter Marheine.
3 comments:
File include/libflashrom.h:
I was trying to find it, but I couldn't, but then I found the addition at `flashrom_set_progress_cal […]
It was not me. the man in the middle :) sorry. fixed
File libflashrom.c:
/* Dynamic messages and double formatting may not be ideal,
as they are slow. It would be better to limit the message
size to a fixed value (e.g., char message[1024] = {0};). */
I found one very long message in cli_classic. […]
Done
File libflashrom.c:
if (!log_callback && global_log_callback == format_message_and_invoke_log_callback_v2)
global_log_callback = NULL;
I spent some time looking at this, then I figured this is doing a reset (when log_callback is passed […]
Great review helps finding bugs. This is the case. Thx
Looks like I had a bad day.
The logic of this condition was to reset log callback v2 only in the case if it was installed by set_v2 op
We have following logic. Better with examples
set_log_cb(cb_v1) /* will set v1 cb and replace v2 if it was set */
set_log_cb_v2(cb_v2) /* set v2 cb and replace v1 */
set_log_cb(NULL) /* reset everything */
set_log_cb_v2(NULL) /* reset v2 cb only. because it should not affect previous API behavior */
The public API must be reliable even if the usage is crazy. This why I wanted to add tests with a mix of both versions of set_log_cb...
The corner cases:
1:
set_log_cb_v2(cb_v2) /* set using v2 */
set_log_cb(NULL) /* reset using v1 */
it was working
2:
set_log_cb(cb_v1) /* set using v1 */
set_log_cb_v2(NULL) /* reset using v2 */
it was an issue. set_v2 did reset of cb_v2 but installed inner cb and replaced v1
My initial implementation was like you suggested. But then I thought my v2 should affect v1 behavior
In your implementation set_v1/v2(NULL) will always reset everything
In my one we will have following
set_log_cb(cb_v1) /* set using v1 */
set_log_cb_v2(NULL) /* will do nothing cb_v1 will continue working */
So I will implement as you will say.
Let's see one more round
To view, visit change 86875. To unsubscribe, or for help writing mail filters, visit settings.