[flashrom-gerrit] [M] Change in flashrom[main]: libflashrom: Update the API for progress callback

[Peter Marheine (Code Review)]

Attention is currently required from: Anastasia Klimchuk, Richard Hughes.

Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/86031?usp=email )

The change is no longer submittable: All-Comments-Resolved is unsatisfied now.

Change subject: libflashrom: Update the API for progress callback
......................................................................

Patch Set 8:

(2 comments)

File doc/release_notes/devel.rst:

https://review.coreboot.org/c/flashrom/+/86031/comment/9b450f33_6693565a?usp... :
PS8, Line 39: New libflashrom API for progress callback
            : -----------------------------------------
            : 
            : API for progress callback has changed. Old one ``flashrom_set_progress_callback`` is deprecated.
            : New API ``flashrom_set_progress_callback_v2`` should be used instead.
            : 
            : The type of callback function has changed as well, old function type ``flashrom_progress_callback``
            : is deprecated, new function type ``flashrom_progress_callback_v2`` should be used instead.
            : 
            : The reason for deprecation is that old API for progress indicator was possible to use only via the hack
            : (like client of API keeping the pointer in a global state), and to allow the normal usage of API a change
            : was needed one way or the other.
Rewording to read better, I think.

```suggestion
New libflashrom API for progress reporting
------------------------------------------

The old ``flashrom_set_progress_callback`` function for requesting progress updates
during library operations is now deprecated. Users should call
``flashrom_set_progress_callback_v2`` instead, which also changes the signature
of the callback function.

This new API fixes limitations with the old one where most users would need to
define their own global state to track progress, and it was impossible to fix that
issue while maintaining binary compatibility without adding a new API.
```

File include/flash.h:

https://review.coreboot.org/c/flashrom/+/86031/comment/39bb9570_ab663f9d?usp... :
PS8, Line 624: 	flashrom_progress_callback_v2 *progress_callback;
             : 	struct flashrom_progress progress_state;
             : 	struct stage_progress stage_progress[FLASHROM_PROGRESS_NR];
             : 	/* deprecated, do not use */
             : 	flashrom_progress_callback *deprecated_progress_callback;
             : 	struct flashrom_progress *deprecated_progress_state;
Could you make this a tagged union, so it's more obviously one or the other and never both? Since this is an internal implementation of a public API, I don't think it makes sense to mark these internals as deprecated because we still want to maintain compatibility.

```

enum progress_kind {
    PROGRESS_V1,
    PROGRESS_V2,
};

struct progress_info_v1 {
    flashrom_progress_callback *callback;
    struct flashrom_progress *state;
}

struct progress_info_v2 {
    flashrom_progress_callback_v2 *callback;
    struct flashrom_progress state;
}

...

enum progress_kind progress_kind;
    union {
        struct progress_info_v1 progress_v1;
        struct progress_info_v2 progress_v2;
    };
    struct stage_progress stage_progress[FLASHROM_PROGRESS_NR];
```