On 07/18/2017 06:24 PM, John Ferlan wrote:
On 06/28/2017 02:49 PM, Cole Robinson wrote:
For the ram/vram monitor wrappers, just add a default: clause... seems like it should be rarely extended so this saves every committer from needing to update
For the validation switch, fill in the missing values
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_domain.c | 3 ++- src/qemu/qemu_monitor_json.c | 16 ++++------------ src/qemu/qemu_process.c | 7 ++----- 3 files changed, 8 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 90f489840..ac1bc1a1e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2950,10 +2950,11 @@ qemuDomainDefValidateVideo(const virDomainDef *def) for (i = 0; i < def->nvideos; i++) { video = def->videos[i];
- switch (video->type) { + switch ((virDomainVideoType) video->type) {
My recollection is when this is typecast or the @type was typed as the enum, then the switch needed every case of the enum to be listed.
Whereas, when the @type was an int, then using 'default:' was possible if one didn't want to provide every possible combination.
It seems to work a bit differently: - If @type is int, no checking is done. - If @type is explicit, gcc checks that either all values are listed, or a default: clause is present
Still, I believe more recent changes have always favored the list every possible case, even if they do nothing rather than using default:
Is there any special reason to not list every case option? If not, I'd prefer _virDomainVideoDef change @type from int to virDomainVideoType if only to avoid this particular type situation in the future.
That said I think it is beneficial to make the VideoDef change and adjust all the users to add an explicit 'default:' if it makes sense. There aren't many cases I can think of outside generic domain_conf code where explicitly listing every VIDEO_TYPE makes sense IMO. There's a bunch of similar cases like that in qemu_domain_address.c but I wonder if that is actually preventing bugs from being added or just saving developers a few minutes hunting through the code... Anyways I'll side step this discussion in my v2 by converting the qemu validation switch to a whitelist approach which makes more sense anyways, and just skipping the (virDomainVideoType) annotation Thanks, Cole