[cp-patches] [RFA] Fix JDWP Values

Kyle Galloway kgallowa at redhat.com
Fri Mar 9 21:27:19 UTC 2007


Keith Seitz wrote:
> [my apologies for the delay in getting back to you]
>
> Kyle Galloway wrote:
>> Keith Seitz wrote:
> >
>>> I see that this method can throw InvalidObjectException if the 
>>> object ID is not known by the ID manager. How is 
>>> JdwpInternalErrorException thrown?
>> This gets thrown by JdwpString.readString().
>
> Ah... Missed that.
>
>> I see what your saying, and I gave it some thought, but there is a 
>> big difference between having the value already packaged in an object 
>> and having to take it out of the byte buffer.  If I were to make a 
>> common method, I would first have to go through like this is the 
>> ByteBuffer case:
>>
>> if (it's an int)
>>  new Integer(bb.getInt)
>> else if (it's a float)
>>  new Float (bb.getFloat)
>> etc.......
>>
>> which is just as much wasted code space, so I think keeping the 
>> object as a special case is the best bet.
>
> Hmm. Yeah. I guess I was hoping generics would help here, but it 
> doesn't really. Oh, well, never mind that.
>
>> I have attached a new patch.
>
> Okay, you saw this coming... :-)
>
>> +  @Override
>> +  /**
>> +   * Return an object representing this type
>> +   * +   * @return an Object represntation of this value
>> +   */
>> +  protected Object getObject()
>> +  {
>> +    return new Integer(_value);
>> +  }
>
> The consensus appears to be to put the annotation just before the 
> method declaration/after the javadoc.
>
>> Index: gnu/classpath/jdwp/value/ValueFactory.java
>> ===================================================================
>> RCS file: gnu/classpath/jdwp/value/ValueFactory.java
>> diff -N gnu/classpath/jdwp/value/ValueFactory.java
>> --- /dev/null    1 Jan 1970 00:00:00 -0000
>> +++ gnu/classpath/jdwp/value/ValueFactory.java    1 Jan 1970 00:00:00 
>> -0000
>> +  public static Value createFromTagged(ByteBuffer bb)
>> +    throws JdwpInternalErrorException, InvalidObjectException
>> +  {
>> +    return create(bb, bb.get());
>> +  }
>
> A little (useless?) paranoia here. Perhaps this method should throw 
> JdwpException, just in case the tag is invalid?
>
>> +  private static Value create(ByteBuffer bb, byte tag)
>> +    throws JdwpInternalErrorException, InvalidObjectException
>> +  {
>> +    Value val = null;
>> +    switch(tag)
>> +    {
> [snip]
>
> I think we should add a default case an throw InvalidTagException 
> (which doesn't exist yet).
>
>> Index: gnu/classpath/jdwp/value/Value.java
>> ===================================================================
>> RCS file: gnu/classpath/jdwp/value/Value.java
>> diff -N gnu/classpath/jdwp/value/Value.java
>> --- /dev/null    1 Jan 1970 00:00:00 -0000
>> +++ gnu/classpath/jdwp/value/Value.java    1 Jan 1970 00:00:00 -0000
>
>> +  /**
>> +   * Return an object representing this type
>
> "Returns"
>
>> +  /**
>> +   * Get an untageed object from the ByteBuffer
>
> "untagged"
>
>> +  /**
>> +   * Get an untageed object from the ByteBuffer
>
> "untagged"
>
> With the above changes, please commit. 
Done and done.

Thanks,
Kyle




More information about the Classpath-patches mailing list