[PATCH v4 09/10] tests: check decoding of NETLINK_SOCK_DIAG AF_INET attribute

[Dmitry V. Levin]

On Mon, Jun 26, 2017 at 10:04:02AM +0800, JingPiao Chen wrote:
> On Mon, Jun 26, 2017 at 02:56:59AM +0300, Dmitry V. Levin wrote:
> > On Tue, Jun 20, 2017 at 05:23:54PM +0800, JingPiao Chen wrote:
> > > * tests/nlattr_inet_diag.c: New file.
> > > * tests/gen_tests.in (nlattr_inet_diag): New entry.
> > > * tests/pure_executables.list: Add nlattr_inet_diag.
> > > * tests/.gitignore: Likewise.
> > 
> > This tests decoding of inet_diag_msg attributes only, please reflect it in
> > commit message.  If you are going to add tests for other inet diag
> > attributes, maybe nlattr_inet_diag should be rather called
> > nlattr_inet_diag_msg.
[...]
> > Please use different addresses for different fields.
> 
> Ok, but I think this test is focus on attribute check.

OK, it's not important then.

[...]
> > > +	dctcp = RTA_DATA(nla);
> > > +	*dctcp = (struct tcp_dctcp_info) {
> > > +		.dctcp_enabled = 0xfdac,
> > > +		.dctcp_ce_state = 0xfadc,
> > > +		.dctcp_alpha = 0xbdabcada,
> > > +		.dctcp_ab_ecn = 0xbadbfafb,
> > > +		.dctcp_ab_tot = 0xfdacdadf
> > > +	};
> > 
> > If you had a source copy of struct tcp_dctcp_info,
> > you'd be able to do a memcpy here, and ...
> > 
> Do you mean that:
> 
> 	struct tcp_dctcp_info dctcp;
> 	dctcp = (struct tcp_dctcp_info) {
> 		.dctcp_enabled = 0xfdac,
> 		.dctcp_ce_state = 0xfadc,
> 		.dctcp_alpha = 0xbdabcada,
> 		.dctcp_ab_ecn = 0xbadbfafb,
> 		.dctcp_ab_tot = 0xfdacdadf
> 	};

I mean

	static const struct tcp_dctcp_info dctcp = {
		.dctcp_enabled = 0xfdac,
		.dctcp_ce_state = 0xfadc,
		.dctcp_alpha = 0xbdabcada,
		.dctcp_ab_ecn = 0xbadbfafb,
		.dctcp_ab_tot = 0xfdacdadf
	};

Here "static const" allows the compiler to place this object
into a readonly memory mapping.

> 	memcpy(RTA_DATA(nla), &dctcp, sizeof(dctcp));
> 
> Other structures should be the same?

Whether to use a SET_STRUCT expression or this kind of initialization
depends on later use.  If the structure is used later in a printf
expression, this helps to avoid duplication of magic constants and therefore
is more preferable.


-- 
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/20170626/fa2bb519/attachment.bin>