[jira] Issue Comment Edited: (THRIFT-882) deep copy of binary fields does not copy ByteBuffer characteristics (arrayOffset, position)

["Mathias Herberts (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-882?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12904944#action_12904944 ] 

Mathias Herberts edited comment on THRIFT-882 at 9/1/10 2:21 AM:
-----------------------------------------------------------------

The handling of non array backed buffers will change any existing mark:

{code}
      orig.mark();
      orig.get(copy.array());
      orig.reset();
{code}

It can be avoided by doing

{code}
  // Save current position
  int pos = orig.position();

  int mark = -1;

  // Save current mark if set
  try {
    orig.reset();
    mark = orig.position();
    orig.position(pos);
  } catch (InvalidMarkException ime) {
  }

  orig.mark();
  orig.get(copy.array());

  // Get rid of mark and reset position to 0.
  orig.rewind();

  // Reset mark to its original value if it was set
  if (-1 != mark) {
    orig.position(mark);
    orig.mark();
  }

  // Reset position to its original value.
  orig.position(pos);
{code}

      was (Author: herberts):
    The handling of non array backed buffers will change any existing mark:


      orig.mark();
      orig.get(copy.array());
      orig.reset();

It can be avoided by doing

  // Save current position
  int pos = orig.position();

  int mark = -1;

  // Save current mark if set
  try {
    orig.reset();
    mark = orig.position();
    orig.position(pos);
  } catch (InvalidMarkException ime) {
  }

  orig.mark();
  orig.get(copy.array());

  // Get rid of mark and reset position to 0.
  orig.rewind();

  // Reset mark to its original value if it was set
  if (-1 != mark) {
    orig.position(mark);
    orig.mark();
  }

  // Reset position to its original value.
  orig.position(pos);

  
> deep copy of binary fields does not copy ByteBuffer characteristics (arrayOffset, position)
> -------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-882
>                 URL: https://issues.apache.org/jira/browse/THRIFT-882
>             Project: Thrift
>          Issue Type: Bug
>          Components: Java - Compiler
>    Affects Versions: 0.4
>            Reporter: Mathias Herberts
>            Assignee: Bryan Duxbury
>             Fix For: 0.5
>
>         Attachments: thrift-882.patch
>
>
> Generated objects have a constructor which does a deep copy of an existing instance.
> For binary fields, the deep copy is done like that:
> {code}
>     if (other.isSetBinary_field()) {
>       this.binary_field = ByteBuffer.wrap(new byte[other.binary_field.limit() - other.binary_field.arrayOffset()]);
>       System.arraycopy(other.binary_field.array(), other.binary_field.arrayOffset(), binary_field.array(), 0, other.binary_field.limit() - other.binary_field.arrayOffset());
>     }
> {code}
> This copies the backing array of the ByteBuffer but does not set the position correctly.
> In various protocol implementations, ByteBuffer instances are serialized by considering data between position and limit, this means that an object created from another object might lead to different serialized data (and thus a different deserialized value) in case a ByteBuffer has a non default position (which can happen since ByteBuffers can be set).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.