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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list