On Thu, 2005-07-21 at 18:18 -0600, Tom Tromey wrote:
> >>>>> "Aaron" == Aaron Luchko <[EMAIL PROTECTED]> writes:
>
> Aaron> There's a slightly ugly part with a big if/else in executeGetValues but
> Aaron> there doesn't seem to be a way to get rid of this.
>
> This looks really similar to some code in the Values patch. I suggest
> a static utility method somewhere that takes a class and returns the
> corresponding tag. Then the big if/else here just becomes a method
> call, and the conditional tag stuff in that method in Values becomes a
> single conditional write... what do you think?
I considered that a bit but decided against it. The code in Value that
does a similar part (writes a tag according to the class) is in
writeValue and is used by both writeTaggedValue and writeUntaggedValue,
and it must both have the option to write the tag (untagged values don't
write tags), and write the value itself. Both these actions depend on
which Class it is, if we had a single method to get tag based on class
we'd still need a switch afterward to write the value for Values.
In order to keep it a single if/else one would need another method in
Value, something like
writeTagAndValue(DataOutputStream os, Object obj, Class type, boolean
tagged, boolean value)
obj would be required since when writing a value the object is required,
type would be for arrayregions since we don't have an object to get the
class from, we'd need to hand it the class directly, tagged is a switch
on whether or not to write the tag for tagged/untagged values and value
is a switch on whether or not to write the obj since we don't want that
for the arrayregions usage (though we could just pass a null object to
signify that instead).
I felt that this added complexity in order to re-use the switch wasn't
worth it since ArrayReference is the only time we use arrayregions in
the entire protocol and I'm pretty sure only time other than Values
we'll need to write out a tag based on a Class. If you feel one of these
solutions is better (or if you think of a better sol'n) I'm glad to
implement it but otherwise I think this approach with the one seemingly
extra if/else ends up being the simplest.
> Aaron> + // Write all the values, primatives should be untagged and
> Objects must be
>
> Should be "primitives" ... this occurs both here and I think in the
> Values patch as well.
Oops, I sometimes have the occasional spellang error :)
patch with gooder spelling attached.
Aaron
--- /dev/null 2005-06-09 16:29:11.371620296 -0400
+++ gnu/classpath/jdwp/processor/ArrayReferenceCommandSet.java 2005-07-22 12:09:17.000000000 -0400
@@ -0,0 +1,178 @@
+/* ArrayReferenceCommandSet.java -- class toclass to implement the Array
+ Reference Command Set
+ Copyright (C) 2005 Free Software Foundation
+
+This file is part of GNU Classpath.
+
+GNU Classpath is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2, or (at your option)
+any later version.
+
+GNU Classpath is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Classpath; see the file COPYING. If not, write to the
+Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+02110-1301 USA.
+
+Linking this library statically or dynamically with other modules is
+making a combined work based on this library. Thus, the terms and
+conditions of the GNU General Public License cover the whole
+combination.
+
+As a special exception, the copyright holders of this library give you
+permission to link this library with independent modules to produce an
+executable, regardless of the license terms of these independent
+modules, and to copy and distribute the resulting executable under
+terms of your choice, provided that you also meet, for each linked
+terms of your choice, provided that you also meet, for each linked
+independent module, the terms and conditions of the license of that
+module. An independent module is a module which is not derived from
+or based on this library. If you modify this library, you may extend
+this exception to your version of the library, but you are not
+obligated to do so. If you do not wish to do so, delete this
+exception statement from your version. */
+
+
+package gnu.classpath.jdwp.processor;
+
+import gnu.classpath.jdwp.Jdwp;
+import gnu.classpath.jdwp.JdwpConstants;
+import gnu.classpath.jdwp.exception.InvalidObjectException;
+import gnu.classpath.jdwp.exception.JdwpException;
+import gnu.classpath.jdwp.exception.JdwpInternalErrorException;
+import gnu.classpath.jdwp.exception.NotImplementedException;
+import gnu.classpath.jdwp.id.IdManager;
+import gnu.classpath.jdwp.id.ObjectId;
+import gnu.classpath.jdwp.util.Value;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.lang.reflect.Array;
+import java.nio.ByteBuffer;
+
+/**
+ * A class representing the ArrayReference Command Set.
+ *
+ * @author Aaron Luchko <[EMAIL PROTECTED]>
+ */
+public class ArrayReferenceCommandSet implements CommandSet
+{
+ // Manages all the different ids that are assigned by jdwp
+ private final IdManager idMan = Jdwp.getIdManager();
+
+ public boolean runCommand(ByteBuffer bb, DataOutputStream os, byte command)
+ throws JdwpException
+ {
+ try
+ {
+ switch (command)
+ {
+ case JdwpConstants.CommandSet.ArrayReference.LENGTH:
+ executeLength(bb, os);
+ break;
+ case JdwpConstants.CommandSet.ArrayReference.GET_VALUES:
+ executeGetValues(bb, os);
+ break;
+ case JdwpConstants.CommandSet.ArrayReference.SET_VALUES:
+ executeSetValues(bb, os);
+ break;
+ default:
+ throw new NotImplementedException("Command " + command +
+ " not found in Array Reference Command Set.");
+ }
+ }
+ catch (IOException ex)
+ {
+ // The DataOutputStream we're using isn't talking to a socket at all
+ // So if we throw an IOException we're in serious trouble
+ throw new JdwpInternalErrorException(ex);
+ }
+ return true;
+ }
+
+ private void executeLength(ByteBuffer bb, DataOutputStream os)
+ throws InvalidObjectException, IOException
+ {
+ ObjectId oid = idMan.readId(bb);
+ Object array = oid.getObject();
+ os.writeInt(Array.getLength(array));
+ }
+
+ private void executeGetValues(ByteBuffer bb, DataOutputStream os)
+ throws JdwpException, IOException
+ {
+ ObjectId oid = idMan.readId(bb);
+ Object array = oid.getObject();
+ int first = bb.getInt();
+ int length = bb.getInt();
+
+ // We need to write out the byte signifying the type of array first
+ Class clazz = array.getClass().getComponentType();
+
+ // Uugh, this is a little ugly but it's the only time we deal with
+ // arrayregions
+ if (clazz == byte.class)
+ os.writeByte(JdwpConstants.Tag.BYTE);
+ else if (clazz == char.class)
+ os.writeByte(JdwpConstants.Tag.CHAR);
+ else if (clazz == float.class)
+ os.writeByte(JdwpConstants.Tag.FLOAT);
+ else if (clazz == double.class)
+ os.writeByte(JdwpConstants.Tag.DOUBLE);
+ else if (clazz == int.class)
+ os.writeByte(JdwpConstants.Tag.BYTE);
+ else if (clazz == long.class)
+ os.writeByte(JdwpConstants.Tag.LONG);
+ else if (clazz == short.class)
+ os.writeByte(JdwpConstants.Tag.SHORT);
+ else if (clazz == void.class)
+ os.writeByte(JdwpConstants.Tag.VOID);
+ else if (clazz == boolean.class)
+ os.writeByte(JdwpConstants.Tag.BOOLEAN);
+ else if (clazz.isArray())
+ os.writeByte(JdwpConstants.Tag.ARRAY);
+ else if (String.class.isAssignableFrom(clazz))
+ os.writeByte(JdwpConstants.Tag.STRING);
+ else if (Thread.class.isAssignableFrom(clazz))
+ os.writeByte(JdwpConstants.Tag.THREAD);
+ else if (ThreadGroup.class.isAssignableFrom(clazz))
+ os.writeByte(JdwpConstants.Tag.THREAD_GROUP);
+ else if (ClassLoader.class.isAssignableFrom(clazz))
+ os.writeByte(JdwpConstants.Tag.CLASS_LOADER);
+ else if (Class.class.isAssignableFrom(clazz))
+ os.writeByte(JdwpConstants.Tag.CLASS_OBJECT);
+ else
+ os.writeByte(JdwpConstants.Tag.OBJECT);
+
+ // Write all the values, primitives should be untagged and Objects must be
+ // tagged
+ for (int i = first; i < first + length; i++)
+ {
+ Object value = Array.get(array, i);
+ if (clazz.isPrimitive())
+ Value.writeUntaggedValue(os, value);
+ else
+ Value.writeTaggedValue(os, value);
+ }
+ }
+
+ private void executeSetValues(ByteBuffer bb, DataOutputStream os)
+ throws IOException, JdwpException
+ {
+ ObjectId oid = idMan.readId(bb);
+ Object array = oid.getObject();
+ int first = bb.getInt();
+ int length = bb.getInt();
+ Class type = array.getClass().getComponentType();
+ for (int i = first; i < first + length; i++)
+ {
+ Object value = Value.getUntaggedObj(bb, type);
+ Array.set(array, i, value);
+ }
+ }
+}
_______________________________________________
Classpath-patches mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/classpath-patches
