[libvirt] [PATCH 1/2] API: make declaration of _LAST enum values conditional

[Jiri Denemark]

On Fri, Jan 20, 2012 at 14:06:26 -0700, Eric Blake wrote:
> Although this is a public API break, it only affects users that
> were compiling against *_LAST values, and can be trivially
> worked around without impacting compilation against older
> headers, by the user defining LIBVIRT_ENUM_SENTINELS before
> including libvirt.h.  It is not an ABI break, since enum values
> do not appear as .so entry points.
> 
> See this list discussion:
> https://www.redhat.com/archives/libvir-list/2012-January/msg00804.html
> 
> * include/libvirt/libvirt.h.in: Hide all sentinels behind
> LIBVIRT_ENUM_SENTINELS, and add missing sentinels.
> * src/internal.h (VIR_DEPRECATED): Allow inclusion after
> libvirt.h.
> (LIBVIRT_ENUM_SENTINELS): Expose sentinels internally.
> * daemon/libvirtd.h: Use the sentinels.
> * src/remote/remote_protocol.x (includes): Don't expose sentinels.
> * python/generator.py (enum): Likewise.
> * tests/cputest.c (cpuTestCompResStr): Silence compiler warning.
> * tools/virsh.c (vshDomainStateReasonToString)
> (vshDomainControlStateToString): Likewise.
> ---
>  daemon/libvirtd.h            |    8 +-
>  include/libvirt/libvirt.h.in |  186 ++++++++++++++++++++++++++++++++++++-----
>  python/generator.py          |    3 +-
>  src/internal.h               |    4 +
>  src/remote/remote_protocol.x |    3 +-
>  tests/cputest.c              |    3 +-
>  tools/virsh.c                |    9 ++
>  7 files changed, 187 insertions(+), 29 deletions(-)

Overall, I like the patch but I don't agree with the way of silencing
compilation warnings in the second part.  The main benefit of avoiding default
in switches used with enums is that the compiler is nice and warns us when new
item is added to an enum.  I'd prefer not to ruin this.

...
> diff --git a/tests/cputest.c b/tests/cputest.c
> index 938e087..2bfac88 100644
> --- a/tests/cputest.c
> +++ b/tests/cputest.c
> @@ -1,7 +1,7 @@
>  /*
>   * cputest.c: Test the libvirtd internal CPU APIs
>   *
> - * Copyright (C) 2010-2011 Red Hat, Inc.
> + * Copyright (C) 2010-2012 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -205,6 +205,7 @@ cpuTestCompResStr(virCPUCompareResult result)
>      case VIR_CPU_COMPARE_INCOMPATIBLE:  return "INCOMPATIBLE";
>      case VIR_CPU_COMPARE_IDENTICAL:     return "IDENTICAL";
>      case VIR_CPU_COMPARE_SUPERSET:      return "SUPERSET";
> +    default: ;
        case VIR_CPU_COMPARE_LAST: ;
>      }
> 
>      return "unknown";
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 032a4e0..fd378a6 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -17574,6 +17574,7 @@ vshDomainStateReasonToString(int state, int reason)
>      case VIR_DOMAIN_NOSTATE:
>          switch ((virDomainNostateReason) reason) {
>          case VIR_DOMAIN_NOSTATE_UNKNOWN:
> +        default:
            case VIR_DOMAIN_NOSTATE_LAST:
>              ;
>          }
>          break;
> @@ -17595,6 +17596,7 @@ vshDomainStateReasonToString(int state, int reason)
>          case VIR_DOMAIN_RUNNING_SAVE_CANCELED:
>              return N_("save canceled");
>          case VIR_DOMAIN_RUNNING_UNKNOWN:
> +        default:
            case VIR_DOMAIN_RUNNING_LAST:
>              ;
>          }
>          break;
> @@ -17602,6 +17604,7 @@ vshDomainStateReasonToString(int state, int reason)
>      case VIR_DOMAIN_BLOCKED:
>          switch ((virDomainBlockedReason) reason) {
>          case VIR_DOMAIN_BLOCKED_UNKNOWN:
> +        default:
            case VIR_DOMAIN_BLOCKED_LAST:
>              ;
>          }
>          break;
> @@ -17625,6 +17628,7 @@ vshDomainStateReasonToString(int state, int reason)
>          case VIR_DOMAIN_PAUSED_SHUTTING_DOWN:
>              return N_("shutting down");
>          case VIR_DOMAIN_PAUSED_UNKNOWN:
> +        default:
            case VIR_DOMAIN_PAUSED_LAST:
>              ;
>          }
>          break;
> @@ -17634,6 +17638,7 @@ vshDomainStateReasonToString(int state, int reason)
>          case VIR_DOMAIN_SHUTDOWN_USER:
>              return N_("user");
>          case VIR_DOMAIN_SHUTDOWN_UNKNOWN:
> +        default:
            case VIR_DOMAIN_SHUTDOWN_LAST:
>              ;
>          }
>          break;
> @@ -17655,6 +17660,7 @@ vshDomainStateReasonToString(int state, int reason)
>          case VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT:
>              return N_("from snapshot");
>          case VIR_DOMAIN_SHUTOFF_UNKNOWN:
> +        default:
            case VIR_DOMAIN_SHUTOFF_LAST:
>              ;
>          }
>          break;
> @@ -17662,6 +17668,7 @@ vshDomainStateReasonToString(int state, int reason)
>      case VIR_DOMAIN_CRASHED:
>          switch ((virDomainCrashedReason) reason) {
>          case VIR_DOMAIN_CRASHED_UNKNOWN:
> +        default:
            case VIR_DOMAIN_CRASHED_LAST:
>              ;
>          }
>          break;
> @@ -17753,6 +17760,8 @@ vshDomainControlStateToString(int state)
>          return N_("occupied");
>      case VIR_DOMAIN_CONTROL_ERROR:
>          return N_("error");
> +    default:
        case VIR_DOMAIN_CONTROL_LAST:
> +        ;
>      }
> 
>      return N_("unknown");

ACK with those defaults removed.

Jirka