[libvirt] [PATCH] [libvirt-java] Fix Array IndexOutOfBoundsException for unknown error codes

[Eric Blake]

On 08/01/2012 08:11 AM, Claudio Bley wrote:
> At Mon, 23 Jul 2012 14:56:55 -0600,

>> the old version crashed, and your version leaves code as null (which is
>> a strict improvement, but might cause its own NullPointer issue later
>> on).  Having an else branch that sticks in a placeholder would be nicer
>> to end clients to at least recognize that they are talking to a newer
>> server, without crashing.
> 
> Please have look at the following patch.
> 
> -- >8 --
> Subject: [PATCH] Fix IndexOutOfBoundsException for unknown error
>  number/domain/level codes.
> 
> Remove default constructor because it does not init the object properly.
> 
> Add a special *_UNKNOWN enum value to each enum which is used when the
> given enum code is out of bounds.

> +    /**
> +     * Map an integer to an enum value.
> +     *
> +     * @return when {@code n < values.length} return n-th item of
> +     *          {@code values}, otherwise the last item of array
> +     *          {@code values}.
> +     */
> +    private static final <T extends Enum<T>> T wrapToEnum(final int n, final T[] values) {

Wow, Java has changed since I last programmed in it (I haven't
programmed Java since 1.4 when 'assert' was added, although I was at
least following the development of Java 1.5 at the time enough to
understand this syntax - I guess that would be 8 or so years ago?).

> @@ -71,7 +90,13 @@ public class Error implements Serializable {
>          /**
>           * An error
>           */
> -        VIR_ERR_ERROR
> +        VIR_ERR_ERROR,
> +
> +        VIR_ERR_UNKNOWN; /* must be the last entry! */

Is this...

> +
> +        protected static final ErrorLevel wrap(int value) {
> +            return wrapToEnum(value, values());
> +        }
>      }
>  
>      public static enum ErrorNumber {
> @@ -161,6 +186,11 @@ public class Error implements Serializable {
>          VIR_ERR_MIGRATE_UNSAFE, /* Migration is not safe */
>          VIR_ERR_OVERFLOW, /* integer overflow */
>          VIR_ERR_BLOCK_COPY_ACTIVE, /* action prevented by block copy job */
> +        VIR_ERR_UNKNOWN; /* unknown error (must be the last entry!) */

...and this common use of a name among two different enums going to bite
us?  If so, I'd recommend munging the names, maybe:
VIR_ERR_LEVEL_UNKNOWN vs. VIR_ERR_NUMBER_UNKNOWN.

>      public Error(virError vError) {
> -        code = ErrorNumber.values()[vError.code];
> -        domain = ErrorDomain.values()[vError.domain];
> -        level = ErrorLevel.values()[vError.level];
> +        code = code.wrap(vError.code);
> +        domain = domain.wrap(vError.domain);
> +        level = level.wrap(vError.level);

Slick.  Depending on your answer to my question on whether a rename
makes sense, I don't mind pushing this or a slightly different v2.  When
using 'git send-email', it helps to use '--subject-prefix="libvirt-java
PATCHv2" ' for the repost to make it obvious that it is a revision of an
earlier post.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120801/b8a727af/attachment-0001.sig>