[M] Change in flashrom[main]: libflashrom: Update the API for Logger Callback

[Dmitry Zhadinets (Code Review)]

Attention is currently required from: Anastasia Klimchuk, Peter Marheine.

Dmitry Zhadinets 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 6:

(3 comments)

File include/libflashrom.h:

https://review.coreboot.org/c/flashrom/+/86875/comment/6eb6957b_0618082a?usp... :
PS4, Line 83:  *
> 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:

https://review.coreboot.org/c/flashrom/+/86875/comment/1d9c4611_5c62c5da?usp... :
PS4, Line 59: 	/* 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:

https://review.coreboot.org/c/flashrom/+/86875/comment/d06b0536_548c8263?usp... :
PS6, Line 84: 	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