Eric Blake <ebl...@redhat.com> writes: > On 06/13/2015 08:20 AM, Markus Armbruster wrote: >> These macros expand into error class enumeration constant, comma, >> string. Unclean. Has been that way since commit 13f59ae. >> >> The error class is always ERROR_CLASS_GENERIC_ERROR since the previous >> commit. >> >> Clean up as follows: >> >> * Prepend every use of a QERR_ macro by ERROR_CLASS_GENERIC_ERROR, and >> delete it from the QERR_ macro. No change after preprocessing. >> >> * Rewrite error_set(ERROR_CLASS_GENERIC_ERROR, ...) into >> error_setg(...). Again, no change after preprocessing. > > Were these conversions done via coccinelle? If so, it would be nice to > mention the script used in the commit message. But that doesn't affect > the correctness of the patch itself.
The semantic patch I used is almost 200 repetitive lines, because I derived it from qerror.h with regexp-magic. I doubt including it is useful. >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- > >> util/qemu-option.c | 22 ++++++++------ >> 53 files changed, 362 insertions(+), 358 deletions(-) > > Close to 1:1 lineup; difference is due to altered line wraps in a few spots. > >> >> diff --git a/backends/rng-egd.c b/backends/rng-egd.c >> index 849bd7a..225221e 100644 >> --- a/backends/rng-egd.c >> +++ b/backends/rng-egd.c >> @@ -140,8 +140,8 @@ static void rng_egd_opened(RngBackend *b, Error **errp) >> RngEgd *s = RNG_EGD(b); >> >> if (s->chr_name == NULL) { >> - error_set(errp, QERR_INVALID_PARAMETER_VALUE, >> - "chardev", "a valid character device"); >> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "chardev", >> + "a valid character device"); > > Interesting line rewrap. Coccinelle. >> +++ b/backends/rng-random.c >> @@ -74,8 +74,8 @@ static void rng_random_opened(RngBackend *b, Error **errp) >> RndRandom *s = RNG_RANDOM(b); >> >> if (s->filename == NULL) { >> - error_set(errp, QERR_INVALID_PARAMETER_VALUE, >> - "filename", "a valid filename"); >> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "filename", >> + "a valid filename"); > > and again > > >> +++ b/block/quorum.c >> @@ -800,8 +800,8 @@ static int quorum_valid_threshold(int threshold, int >> num_children, Error **errp) >> { >> >> if (threshold < 1) { >> - error_set(errp, QERR_INVALID_PARAMETER_VALUE, >> - "vote-threshold", "value >= 1"); >> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "vote-threshold", >> + "value >= 1"); > > looks like there are several of them. They don't hurt, so I'll quit > pointing them out. I'll try to avoid uncalled-for rewraps in v2. >> #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \ >> - ERROR_CLASS_GENERIC_ERROR, "Property %s.%s doesn't take value %" >> PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")" >> + "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 >> ", maximum: %" PRId64 ")" > > Still a long line; worth wrapping with another \ mid-string? Normally yes, but these macros should die, hopefully soon. >> +++ b/tpm.c >> @@ -140,20 +140,22 @@ static int configure_tpm(QemuOpts *opts) >> >> id = qemu_opts_id(opts); >> if (id == NULL) { >> - qerror_report(QERR_MISSING_PARAMETER, "id"); >> + qerror_report(ERROR_CLASS_GENERIC_ERROR, QERR_MISSING_PARAMETER, >> "id"); >> return 1; > > Not quite the same s/error_set/error_setg/ change as everywhere else, > but still correct. > > There may be additional merge conflicts depending on timing of getting > this in, but it is mechanical resolution, and the sooner we get this in, > the better. Yup. > Reviewed-by: Eric Blake <ebl...@redhat.com> Thanks!