| Message ID | 20200721131911.27380-2-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/nvram/fw_cfg: Let fw_cfg_add_from_generator() return boolean value | expand |
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > Document FWCfgDataGeneratorClass::get_data() return NULL > on error, and non-NULL on success. This allow us to simplify > fw_cfg_add_from_generator(). Since we don't need a local > variable to propagate the error, we can remove the ERRP_GUARD() > macro. > > Suggested-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/hw/nvram/fw_cfg.h | 4 +++- > hw/nvram/fw_cfg.c | 3 +-- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h > index 11feae3177..bbcf405649 100644 > --- a/include/hw/nvram/fw_cfg.h > +++ b/include/hw/nvram/fw_cfg.h > @@ -32,7 +32,9 @@ typedef 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. > + * Returns: reference to a byte array containing the data on success, > + * or NULL on error. > + * > * The caller should release the reference when no longer > * required. > */ Thanks for clarifying the doc comment! As far as I can tell, the only implementation is qcrypto_tls_cipher_suites_fw_cfg_gen_data(), which is a trivial wrapper around qcrypto_tls_cipher_suites_get_data(). I checked it conforms. > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 3b1811d3bf..dfa1f2012a 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -1035,7 +1035,6 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, > void fw_cfg_add_from_generator(FWCfgState *s, const char *filename, > const char *gen_id, Error **errp) > { > - ERRP_GUARD(); > FWCfgDataGeneratorClass *klass; > GByteArray *array; > Object *obj; > @@ -1053,7 +1052,7 @@ void fw_cfg_add_from_generator(FWCfgState *s, const char *filename, > } > klass = FW_CFG_DATA_GENERATOR_GET_CLASS(obj); > array = klass->get_data(obj, errp); > - if (*errp) { > + if (!array) { > return; > } > size = array->len; Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h index 11feae3177..bbcf405649 100644 --- a/include/hw/nvram/fw_cfg.h +++ b/include/hw/nvram/fw_cfg.h @@ -32,7 +32,9 @@ typedef 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. + * Returns: reference to a byte array containing the data on success, + * or NULL on error. + * * The caller should release the reference when no longer * required. */ diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 3b1811d3bf..dfa1f2012a 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -1035,7 +1035,6 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void fw_cfg_add_from_generator(FWCfgState *s, const char *filename, const char *gen_id, Error **errp) { - ERRP_GUARD(); FWCfgDataGeneratorClass *klass; GByteArray *array; Object *obj; @@ -1053,7 +1052,7 @@ void fw_cfg_add_from_generator(FWCfgState *s, const char *filename, } klass = FW_CFG_DATA_GENERATOR_GET_CLASS(obj); array = klass->get_data(obj, errp); - if (*errp) { + if (!array) { return; } size = array->len;
Document FWCfgDataGeneratorClass::get_data() return NULL on error, and non-NULL on success. This allow us to simplify fw_cfg_add_from_generator(). Since we don't need a local variable to propagate the error, we can remove the ERRP_GUARD() macro. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/nvram/fw_cfg.h | 4 +++- hw/nvram/fw_cfg.c | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-)