Eric Blake <ebl...@redhat.com> writes: > On 4/23/20 11:00 AM, Markus Armbruster wrote: >> output_type_enum() fails when *obj is not a valid value of the enum >> type. Should not happen. Drop the check, along with its unit tests. >> qapi_enum_lookup()'s assertion still guards. >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> qapi/qapi-visit-core.c | 9 ------- >> tests/test-qobject-output-visitor.c | 39 ----------------------------- >> tests/test-string-output-visitor.c | 19 -------------- >> 3 files changed, 67 deletions(-) > > Nice cleanup. > > The commit message implies adding an assertion; but in reality, the > change is deleting dead code (because we already have the assertion in > place). Maybe it's worth updating the subject?
I tried to say it in the body: "qapi_enum_lookup()'s assertion still guards." I could replace that by "This unmasks qapi_enum_lookup()'s assertion." Okay? Better ideas? > Reviewed-by: Eric Blake <ebl...@redhat.com> > >> - /* >> - * TODO why is this an error, not an assertion? If assertion: >> - * delete, and rely on qapi_enum_lookup() >> - */ >> - if (value < 0 || value >= lookup->size) { >> - error_setg(errp, QERR_INVALID_PARAMETER, name ? name : "null"); >> - return; >> - } > > The comment being deleted is what points out the assertion that > already exists. Thanks!