[libvirt] [PATCH v3] [libvirt-java] Fix Array IndexOutOfBoundsException for unknown error codes
Claudio Bley
cbley at av-test.de
Thu Aug 9 13:55:02 UTC 2012
At Wed, 01 Aug 2012 16:57:33 -0600,
Eric Blake wrote:
>
> 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?).
Indeed, they try to shoehorn every kind of feature into the language,
nowadays. ;)
But, as I'm looking at the code now, I notice that it is unnecessarily
complex (originally I was trying to make it more succinct, but Java
wouldn't let me).
I simplified the code, renaming the method / fixing the documentation
and made v3 of the patch.
> > @@ -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?
No, not at all. Enums are type-safe in Java and enums have their own
namespace; one always has to specify the enum name when referring to
an enum constant. (except in case statements where the compiler can
infer the type of the enum constant)
Rather, I would /very/ much like these superfluous prefixes of all enum
constants to be removed from the libvirt-java interface. They're of no
use really, despite adding to the code bloat.
> 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.
OK, thanks for the tip.
-- >8 --
Subject: [libvirt-java PATCHv3] 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.
---
src/main/java/org/libvirt/Error.java | 43 +++++++++++++++++++++++++++++-------
1 file changed, 35 insertions(+), 8 deletions(-)
diff --git a/src/main/java/org/libvirt/Error.java b/src/main/java/org/libvirt/Error.java
index f185ce0..e3c3246 100644
--- a/src/main/java/org/libvirt/Error.java
+++ b/src/main/java/org/libvirt/Error.java
@@ -12,6 +12,21 @@ import org.libvirt.jna.virError;
*/
public class Error implements Serializable {
+ /**
+ * Returns the element of the given array at the specified index,
+ * or the last element of the array if the index is not less than
+ * {@code values.length}.
+ *
+ * @return n-th item of {@code values} when {@code n <
+ * values.length}, otherwise the last item of {@code values}.
+ */
+ private static final <T> T safeElementAt(final int n, final T[] values) {
+ assert(n >= 0 && values.length > 0);
+
+ int idx = Math.min(n, values.length - 1);
+ return values[idx];
+ }
+
public static enum ErrorDomain {
VIR_FROM_NONE, VIR_FROM_XEN, /* Error at Xen hypervisor layer */
VIR_FROM_XEND, /* Error at connection with xend daemon */
@@ -60,6 +75,11 @@ public class Error implements Serializable {
VIR_FROM_URI, /* Error from URI handling */
VIR_FROM_AUTH, /* Error from auth handling */
VIR_FROM_DBUS, /* Error from DBus */
+ VIR_FROM_UNKNOWN; /* unknown error domain (must be the last entry!) */
+
+ protected static final ErrorDomain wrap(int value) {
+ return safeElementAt(value, values());
+ }
}
public static enum ErrorLevel {
@@ -71,7 +91,13 @@ public class Error implements Serializable {
/**
* An error
*/
- VIR_ERR_ERROR
+ VIR_ERR_ERROR,
+
+ VIR_ERR_UNKNOWN; /* must be the last entry! */
+
+ protected static final ErrorLevel wrap(int value) {
+ return safeElementAt(value, values());
+ }
}
public static enum ErrorNumber {
@@ -161,6 +187,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!) */
+
+ protected static final ErrorNumber wrap(int value) {
+ return safeElementAt(value, values());
+ }
}
/**
@@ -181,14 +212,10 @@ public class Error implements Serializable {
int int2;
NetworkPointer VNP; /* Deprecated */
- public Error() {
-
- }
-
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);
message = vError.message;
str1 = vError.str1;
str2 = vError.str2;
--
1.7.11.msysgit.0
--
AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
Phone: +49 341 265 310 19
Web:<http://www.av-test.org>
Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
More information about the libvir-list
mailing list