[PATCH] tests: check decoding of NETLINK_SOCK_DIAG AF_UNIX messages

[Dmitry V. Levin]

On Wed, Jun 14, 2017 at 03:01:11PM +0800, JingPiao Chen wrote:
> 2017-06-14 12:35 GMT+08:00 Dmitry V. Levin <ldv at altlinux.org>:
> > On Wed, Jun 14, 2017 at 11:53:02AM +0800, JingPiao Chen wrote:
> > > On Wed, Jun 14, 2017 at 05:06:06AM +0300, Dmitry V. Levin wrote:
> > > > On Wed, Jun 14, 2017 at 03:19:46AM +0300, Dmitry V. Levin wrote:
[...]
> > > > > I see.  Would it be better to print the address instead of the
> string
> > > > > in case of umove failure, e.g.
> > > > >
> > > > > if (len < sizeof(msg))
> > > > > return false;
> > > > >
> > > > > if (umove_or_printaddr(tcp, addr, &msg))
> > > > > return true;
> > > >
> > > > OK, I've changed this parser a bit and pushed it again to ldv/netlink
> > > > branch, please have a look.
> > > Ok, thank you.
> >
> > ... and again, amending the last commit to simplify subsequent
> > netlink_sock_diag test changes.
> 
> I have some questions, why do not use:
> if (len < sizeof(msg))
> return false;
> 
> if (umove_or_printaddr(tcp, addr, &msg))
> return true;
> 
> Can we remove offset?

These two code snippets are almost equivalent:

	const size_t offset = sizeof(msg.udiag_family);
	if (!umoven_or_printaddr(tcp, addr + offset,
				 sizeof(msg) - offset,
				 (void *) &msg + offset)) {

and

	if (umove(tcp, addr, &msg) < 0)
		printaddr(addr + sizeof(msg.udiag_family));
	else {

except the second one fetches msg.udiag_family for the second time.
Why would you prefer it over the first one?

[..]
> Can we print address when len < sizeof(msg)?
> Why print ... when len < sizeof(msg)?

How would you distinguish the case when the specified length was correct
but the fetch failed from the case when the specified length was wrong?


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