| Message ID | 20240229143914.1977550-15-zhao1.liu@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Cleanup up to fix missing ERRP_GUARD() for error_prepend() | expand |
Zhao Liu <zhao1.liu@linux.intel.com> writes: > From: Zhao Liu <zhao1.liu@intel.com> > > As the comment in qapi/error, passing @errp to error_prepend() requires > ERRP_GUARD(): > > * = Why, when and how to use ERRP_GUARD() = > * > * Without ERRP_GUARD(), use of the @errp parameter is restricted: > ... > * - It should not be passed to error_prepend(), error_vprepend() or > * error_append_hint(), because that doesn't work with &error_fatal. > * ERRP_GUARD() lifts these restrictions. > * > * To use ERRP_GUARD(), add it right at the beginning of the function. > * @errp can then be used without worrying about the argument being > * NULL or &error_fatal. > > ERRP_GUARD() could avoid the case when @errp is the pointer of > error_fatal, the user can't see this additional information, because > exit() happens in error_setg earlier than information is added [1]. > > The migrate_params_check() passes @errp to error_prepend() without > ERRP_GUARD(), and it could be called from migration_object_init(), > where the passed @errp points to @error_fatal. > > Therefore, the error message echoed in error_prepend() will be lost > because of the above issue. > > To fix this, add missing ERRP_GUARD() at the beginning of this function. > > [1]: Issue description in the commit message of commit ae7c80a7bd73 > ("error: New macro ERRP_GUARD()"). > > Cc: Peter Xu <peterx@redhat.com> > Cc: Fabiano Rosas <farosas@suse.de> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> Reviewed-by: Fabiano Rosas <farosas@suse.de>
On Thu, Feb 29, 2024 at 10:39:11PM +0800, Zhao Liu wrote: > From: Zhao Liu <zhao1.liu@intel.com> > > As the comment in qapi/error, passing @errp to error_prepend() requires > ERRP_GUARD(): > > * = Why, when and how to use ERRP_GUARD() = > * > * Without ERRP_GUARD(), use of the @errp parameter is restricted: > ... > * - It should not be passed to error_prepend(), error_vprepend() or > * error_append_hint(), because that doesn't work with &error_fatal. > * ERRP_GUARD() lifts these restrictions. > * > * To use ERRP_GUARD(), add it right at the beginning of the function. > * @errp can then be used without worrying about the argument being > * NULL or &error_fatal. > > ERRP_GUARD() could avoid the case when @errp is the pointer of > error_fatal, the user can't see this additional information, because > exit() happens in error_setg earlier than information is added [1]. > > The migrate_params_check() passes @errp to error_prepend() without > ERRP_GUARD(), and it could be called from migration_object_init(), > where the passed @errp points to @error_fatal. > > Therefore, the error message echoed in error_prepend() will be lost > because of the above issue. > > To fix this, add missing ERRP_GUARD() at the beginning of this function. > > [1]: Issue description in the commit message of commit ae7c80a7bd73 > ("error: New macro ERRP_GUARD()"). > > Cc: Peter Xu <peterx@redhat.com> > Cc: Fabiano Rosas <farosas@suse.de> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> Acked-by: Peter Xu <peterx@redhat.com>
diff --git a/migration/options.c b/migration/options.c index 3e3e0b93b439..b98cd639979e 100644 --- a/migration/options.c +++ b/migration/options.c @@ -1057,6 +1057,8 @@ void migrate_params_init(MigrationParameters *params) */ bool migrate_params_check(MigrationParameters *params, Error **errp) { + ERRP_GUARD(); + if (params->has_compress_level && (params->compress_level > 9)) { error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "compress_level",