| Message ID | 20241213133352.10915-4-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/nvram/fw_cfg: Move PCI bus methods out | expand |
On Fri, Dec 13, 2024 at 02:33:48PM +0100, Philippe Mathieu-Daudé wrote: > Allow the FW_CFG_DATA_GENERATOR interface get_data() handler to > return NULL when there is nothing to generate. In that case > fw_cfg_add_file_from_generator() will not add any item and > return %true. > > Reported-by: Daniel P. Berrangé <berrange@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/hw/nvram/fw_cfg.h | 13 ++++++++----- > hw/nvram/fw_cfg.c | 10 ++++++---- > 2 files changed, 14 insertions(+), 9 deletions(-) > > diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h > index fcb06f18cc3..5211018fd8f 100644 > --- a/include/hw/nvram/fw_cfg.h > +++ b/include/hw/nvram/fw_cfg.h > @@ -30,8 +30,9 @@ struct FWCfgDataGeneratorClass { > * @obj: the object implementing this interface > * @errp: pointer to a NULL-initialized error object > * > - * Returns: reference to a byte array containing the data on success, > - * or NULL on error. > + * Returns: NULL on failure (errp set if not NULL). > + * A byte array containing the data (if any, > + * otherwise NULL) on success. Bit confusing wording, lets say Returns: A byte array containing data to add, or NULL without @errp set if no data is required, or NULL with @errp set on failure. With that change: Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel
On 13/12/24 14:48, Daniel P. Berrangé wrote: > On Fri, Dec 13, 2024 at 02:33:48PM +0100, Philippe Mathieu-Daudé wrote: >> Allow the FW_CFG_DATA_GENERATOR interface get_data() handler to >> return NULL when there is nothing to generate. In that case >> fw_cfg_add_file_from_generator() will not add any item and >> return %true. >> >> Reported-by: Daniel P. Berrangé <berrange@redhat.com> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> include/hw/nvram/fw_cfg.h | 13 ++++++++----- >> hw/nvram/fw_cfg.c | 10 ++++++---- >> 2 files changed, 14 insertions(+), 9 deletions(-) >> >> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h >> index fcb06f18cc3..5211018fd8f 100644 >> --- a/include/hw/nvram/fw_cfg.h >> +++ b/include/hw/nvram/fw_cfg.h >> @@ -30,8 +30,9 @@ struct FWCfgDataGeneratorClass { >> * @obj: the object implementing this interface >> * @errp: pointer to a NULL-initialized error object >> * >> - * Returns: reference to a byte array containing the data on success, >> - * or NULL on error. >> + * Returns: NULL on failure (errp set if not NULL). >> + * A byte array containing the data (if any, >> + * otherwise NULL) on success. > > Bit confusing wording, lets say > > Returns: A byte array containing data to add, or NULL without > @errp set if no data is required, or NULL with @errp > set on failure. Thank you! > > With that change: > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > With regards, > Daniel
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h index fcb06f18cc3..5211018fd8f 100644 --- a/include/hw/nvram/fw_cfg.h +++ b/include/hw/nvram/fw_cfg.h @@ -30,8 +30,9 @@ struct FWCfgDataGeneratorClass { * @obj: the object implementing this interface * @errp: pointer to a NULL-initialized error object * - * Returns: reference to a byte array containing the data on success, - * or NULL on error. + * Returns: NULL on failure (errp set if not NULL). + * A byte array containing the data (if any, + * otherwise NULL) on success. * * The caller should release the reference when no longer * required. @@ -298,14 +299,16 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data, * @parent: the object in which to resolve the @part * @errp: pointer to a NULL initialized error object * - * Add a new NAMED fw_cfg item with the content generated from the - * @part object. The data generated by the @part object is copied - * into the data structure of the fw_cfg device. + * If the @part object generates content, add a new NAMED fw_cfg item with it. + * The data generated by the @part object is copied into the data structure of + * the fw_cfg device. * The next available (unused) selector key starting at FW_CFG_FILE_FIRST * will be used; also, a new entry will be added to the file directory * structure residing at key value FW_CFG_FILE_DIR, containing the item name, * data size, and assigned selector key value. * + * If the @part object does not generate content, no fw_cfg item is added. + * * Returns: %true on success, %false on error. */ bool fw_cfg_add_file_from_generator(FWCfgState *s, diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index b94cd27bd85..7e1065e5f50 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -1031,10 +1031,10 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s, Object *parent, const char *part, const char *filename, Error **errp) { + ERRP_GUARD(); FWCfgDataGeneratorClass *klass; GByteArray *array; Object *obj; - gsize size; obj = object_resolve_path_component(parent, part); if (!obj) { @@ -1048,11 +1048,13 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s, } klass = FW_CFG_DATA_GENERATOR_GET_CLASS(obj); array = klass->get_data(obj, errp); - if (!array) { + if (*errp) { return false; } - size = array->len; - fw_cfg_add_file(s, filename, g_byte_array_free(array, FALSE), size); + if (array) { + fw_cfg_add_file(s, filename, g_byte_array_free(array, FALSE), + array->len); + } return true; }
Allow the FW_CFG_DATA_GENERATOR interface get_data() handler to return NULL when there is nothing to generate. In that case fw_cfg_add_file_from_generator() will not add any item and return %true. Reported-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/nvram/fw_cfg.h | 13 ++++++++----- hw/nvram/fw_cfg.c | 10 ++++++---- 2 files changed, 14 insertions(+), 9 deletions(-)