[prev in list] [next in list] [prev in thread] [next in thread] 

List:       strace
Subject:    Re: [PATCH 2/3] Add decoding for binder command/return protocol
From:       "Dmitry V. Levin" <ldv () altlinux ! org>
Date:       2016-05-30 13:52:17
Message-ID: 20160530135217.GD2913 () altlinux ! org
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


On Sat, May 28, 2016 at 10:40:27PM +0200, Antoine Damhet wrote:
> This patch finishes the decoding of the content of
> binder_driver_return_protocol and binder_driver_command_protocol.

I'll skip issues similar to the 1st part and comment the rest.

[...]
> +static int
> +decode_binder_transaction_buffer(struct tcb *tcp, struct binder_transaction_data *tr)
> +{
> +	binder_size_t *off = malloc(tr->offsets_size);
> +	if (!off)
> +		return 1;
> +
> +	if (umoven(tcp, tr->data.ptr.offsets, tr->offsets_size, off)) {
> +		free(off);
> +		return 1;
> +	}
> +
> +	char *buf = malloc(tr->data_size);
> +	if (!buf) {
> +		free(off);
> +		return 1;
> +	}
> +
> +	if (umoven(tcp, tr->data.ptr.buffer, tr->data_size, buf)) {
> +		free(off);
> +		free(buf);
> +		return 1;
> +	}
> +
> +	binder_size_t *end = (void *)((char *)off + tr->offsets_size);
> +	binder_size_t *i = off;
> +
> +	goto print_one_transaction;
> +	for (; i < end; ++i) {
> +		tprints(", ");
> +print_one_transaction:
> +		decode_flat_binder_object((struct flat_binder_object *)(buf + *i));
> +	}
> +
> +	free(off);
> +	free(buf);
> +	return 0;
> +}

Decoding arrays by hand has proven to be hard so I recently added print_array() API,
please change this parser to use print_array.

> +static int
> +decode_binder_commands_parameters(struct tcb *tcp, uint32_t type, char *data)

It's more appropriate to define data as void *.

> +static int
> +decode_binder_returns_parameters(struct tcb *tcp, uint32_t type, char *data)

Likewise.

> @@ -48,14 +275,10 @@ decode_binder_returns(struct tcb *tcp, struct binder_write_read *wr)
>  
>  print_one_read_buffer:
>  		type = *(uint32_t *)(buffer + pos);
> -		if (_IOC_SIZE(type) > 0) {
> -			tprints("[");
> -			printxval(binder_driver_returns, type, "BR_???");
> -			tprints(", ");
> -			print_quoted_string(buffer + pos + sizeof(type),
> -					_IOC_SIZE(type), 0);
> -			tprints("]");
> -		} else
> +		if (_IOC_SIZE(type) > 0)
> +			decode_binder_returns_parameters(tcp, type,
> +					buffer + pos + sizeof(uint32_t));
> +		else

There has to be a check that _IOC_SIZE(type) is large enough for the type,
most likely in each printer since they already know the size required.

>  			printxval(binder_driver_returns, type, "BR_???");
>  		pos += sizeof(uint32_t) + _IOC_SIZE(type);
>  	}
> @@ -97,14 +320,10 @@ decode_binder_commands(struct tcb *tcp, struct binder_write_read *wr)
>  
>  print_one_write_buffer:
>  		type = *(uint32_t *)(buffer + pos);
> -		if (_IOC_SIZE(type) > 0) {
> -			tprints("[");
> -			printxval(binder_driver_commands, type, "BC_???");
> -			tprints(", ");
> -			print_quoted_string(buffer + pos + sizeof(type),
> -					_IOC_SIZE(type), 0);
> -			tprints("]");
> -		} else
> +		if (_IOC_SIZE(type) > 0)
> +			decode_binder_commands_parameters(tcp, type,
> +					buffer + pos + sizeof(uint32_t));
> +		else

Likewise.


-- 
ldv

[Attachment #5 (application/pgp-signature)]

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e

_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel


[prev in list] [next in list] [prev in thread] [next in thread]