[PATCH v4 2/6] Add a enumerate decoder to tprint_iov() and tprint_iov_upto().

[Dmitry V. Levin]

On Fri, Jun 17, 2016 at 04:29:54PM +0000, Fabien Siron wrote:
> This commit introduces a new type iov_decoder which will be used instead
> of as a decoder parameter.
> 
> * defs.h (iov_decoder): New enum.
> (tprint_iov, tprint_iov_upto): Change type of "decode_iov" to enum iov_decoder.
> * aio.c (print_iocb): Change type of "decode_iov" to enum iov_decoder in
> tprint_iov() call.
> * keyctl.c (keyctl_instantiate_key_iov): Likewise.
> * process.c (ptrace): Likewise.
> * process_vm.c (process_vm_readv, process_vm_writev): Likewise.
> * io.c (writev, do_pwritev, vmsplice): Likewise.
> (print_iovec): Replace the condition with a switch.
> (tprint_iov_upto): Change type of "decode_iov" to enum iov_decoder.
> (readv, do_preadv): Change type of "decode_iov" to enum iov_decoder in
> tprint_iov_upto() call.
> * scsi.c (print_sg_io_v3_req, print_sg_io_v3_res, print_sg_io_v4_req,
> print_sg_io_v4_res): Likewise.
> ---
>  aio.c        |  4 +++-
>  defs.h       | 10 ++++++++--
>  io.c         | 40 ++++++++++++++++++++++++----------------
>  keyctl.c     |  2 +-
>  net.c        |  3 ++-
>  process.c    |  6 ++++--
>  process_vm.c | 10 ++++++----
>  scsi.c       | 10 ++++++----
>  8 files changed, 54 insertions(+), 31 deletions(-)
> 
> diff --git a/aio.c b/aio.c
> index e02af7d..83a29fc 100644
> --- a/aio.c
> +++ b/aio.c
> @@ -140,7 +140,9 @@ print_iocb(struct tcb *tcp, const struct iocb *cb)
>  		if (iocb_is_valid(cb)) {
>  			tprints(", iovec=");
>  			tprint_iov(tcp, cb->aio_nbytes, cb->aio_buf,
> -				   cb->aio_lio_opcode == 8);
> +				   cb->aio_lio_opcode == 8
> +				   ? IOV_DECODER_STR
> +				   : IOV_DECODER_ADDR);
>  		} else {
>  			tprintf(", buf=%#" PRIx64 ", nbytes=%" PRIu64,
>  				(uint64_t) cb->aio_buf,
> diff --git a/defs.h b/defs.h
> index 311fccd..a86c78c 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -442,6 +442,11 @@ enum sock_proto {
>  };
>  extern enum sock_proto get_proto_by_name(const char *);
>  
> +enum iov_decoder {
> +	IOV_DECODER_ADDR,
> +	IOV_DECODER_STR
> +};

decoder is an actor, it suits more to a function that does actual
decoding.  If I was choosing a name of an option, I'd rather use decode.
Let's change iov_decoder to iov_decode and IOV_DECODER_* to IOV_DECODE_*.

> +
>  typedef enum {
>  	CFLAG_NONE = 0,
>  	CFLAG_ONLY_STATS,
> @@ -659,8 +664,9 @@ extern const char *sprintsigmask_n(const char *, const void *, unsigned int);
>  #define tprintsigmask_addr(prefix, mask) \
>  	tprints(sprintsigmask_n((prefix), (mask), sizeof(mask)))
>  extern void printsignal(int);
> -extern void tprint_iov(struct tcb *, unsigned long, unsigned long, int decode_iov);
> -extern void tprint_iov_upto(struct tcb *, unsigned long, unsigned long, int decode_iov, unsigned long);
> +extern void tprint_iov(struct tcb *, unsigned long, unsigned long, enum iov_decoder);
> +extern void tprint_iov_upto(struct tcb *, unsigned long, unsigned long,
> +			    enum iov_decoder, unsigned long);
>  extern void tprint_open_modes(unsigned int);
>  extern const char *sprint_open_modes(unsigned int);
>  extern void print_seccomp_filter(struct tcb *, unsigned long);
> diff --git a/io.c b/io.c
> index 87b5f47..ad92d84 100644
> --- a/io.c
> +++ b/io.c
> @@ -79,14 +79,21 @@ print_iovec(struct tcb *tcp, void *elem_buf, size_t elem_size, void *data)
>  
>  	tprints("{");
>  
> -	if (c->decode_iov) {
> -		unsigned long len = iov[1];
> -		if (len > c->data_size)
> -			len = c->data_size;
> -		c->data_size -= len;
> -		printstr(tcp, iov[0], len);
> -	} else {
> -		printaddr(iov[0]);
> +	switch (c->decode_iov) {
> +		case IOV_DECODER_STR:
> +		{
> +			unsigned long len = iov[1];
> +			if (len > c->data_size)
> +				len = c->data_size;
> +			c->data_size -= len;
> +			printstr(tcp, iov[0], len);
> +			break;
> +		}
> +		default:
> +		{
> +			printaddr(iov[0]);
> +			break;
> +		}
>  	}
>  
>  	tprintf(", %lu}", iov[1]);
> @@ -100,7 +107,7 @@ print_iovec(struct tcb *tcp, void *elem_buf, size_t elem_size, void *data)
>   */
>  void
>  tprint_iov_upto(struct tcb *tcp, unsigned long len, unsigned long addr,
> -		int decode_iov, unsigned long data_size)
> +		enum iov_decoder decode_iov, unsigned long data_size)
>  {
>  	unsigned long iov[2];
>  	struct print_iovec_config config =

Note that struct print_iovec_config also has to be updated.

[...]
> --- a/process.c
> +++ b/process.c
> @@ -201,7 +201,8 @@ SYS_FUNC(ptrace)
>  			print_sigset_addr_len(tcp, data, addr);
>  			break;
>  		case PTRACE_SETREGSET:
> -			tprint_iov(tcp, /*len:*/ 1, data, /*as string:*/ 0);
> +			tprint_iov(tcp, /*len:*/ 1, data,
> +				   /*as string:*/ IOV_DECODER_ADDR);

Use of IOV_DECODER_ADDR makes this /*as string:*/ comment confusing,
let's remove it.

>  			break;
>  #ifndef IA64
>  		case PTRACE_PEEKDATA:
> @@ -238,7 +239,8 @@ SYS_FUNC(ptrace)
>  			printnum_ulong(tcp, data);
>  			break;
>  		case PTRACE_GETREGSET:
> -			tprint_iov(tcp, /*len:*/ 1, data, /*as string:*/ 0);
> +			tprint_iov(tcp, /*len:*/ 1, data,
> +				   /*as string:*/ IOV_DECODER_ADDR);

Likewise.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160620/c2ef1ea8/attachment.bin>