Re: [PATCH v2 19/19] read/write_pieced_value: Merge into one function

[Yao Qi <qiyaoltc at gmail dot com>]

Andreas Arnez <arnez@linux.vnet.ibm.com> writes:

> +	    if (from == NULL)
> +	      {
> +		/* Read mode.  */
> +		read_value_memory (v, offset,
> +				   p->v.mem.in_stack_memory,
> +				   p->v.mem.addr + bits_to_skip / 8,
> +				   buffer.data (), this_size);
> +		copy_bitwise (v_contents, offset,
> +			      buffer.data (), bits_to_skip % 8,
> +			      this_size_bits, bits_big_endian);
> +		break;
> +	      }
> +
> +	    /* Write mode.  */

I feel it is more clear to add "else" here, like

  if (from == NULL)
    {
      /* Read mode.  */
    }
  else
    {
      /* Write mode.  */
    }

then, we don't need the "break" above.

> +	    if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
> +	      {
> +		if (this_size <= 8)
> +		  {
> +		    /* Perform a single read for small sizes.  */
> +		    read_memory (start_addr, buffer.data (), this_size);
> +		  }
> +		else
> +		  {
> +		    /* Only the first and last bytes can possibly have any
> +		       bits reused.  */
> +		    read_memory (start_addr, buffer.data (), 1);
> +		    read_memory (start_addr + this_size - 1,
> +				 &buffer[this_size - 1], 1);
> +		  }
> +	      }
> +
> +	    copy_bitwise (buffer.data (), bits_to_skip % 8,
> +			  from_contents, offset,
> +			  this_size_bits, bits_big_endian);
> +	    write_memory_with_notification (start_addr, buffer.data (),
> +					    this_size);
> +	  }
>  	  break;

Otherwise, the patch is good to me.

-- 
Yao (齐尧)