On Tue, Jun 27, 2017 at 06:10:10PM +0800, JingPiao Chen wrote:
[...]
> + if (!inet_pton(AF_INET, address, msg->id.idiag_src))
> + perror_msg_and_skip("inet_pton");
> + if (!inet_pton(AF_INET, address, msg->id.idiag_dst))
> + perror_msg_and_skip("inet_pton");
> +}
This can be shortened as I wrote for the previous patch.
[...]
> +static void
> +test_nla_str(const int fd)
> +{
> + const int hdrlen = sizeof(struct inet_diag_msg);
> + const char address[] = "12.34.56.78";
> + struct nlmsghdr *nlh;
> + struct nlattr *nla;
> + unsigned int nla_len;
> + unsigned int msg_len;
> + void *const nlh0 = tail_alloc(NLMSG_SPACE(hdrlen));
> + long rc;
> +
> + nla_len = NLA_HDRLEN + 4;
I suppose this magic constant 4 ...
> + msg_len = NLMSG_SPACE(hdrlen) + nla_len;
> + nlh = nlh0 - nla_len;
> + init_inet_diag_msg(nlh, msg_len, address);
> +
> + nla = NLMSG_ATTR(nlh, hdrlen);
> + SET_STRUCT(struct nlattr, nla,
> + .nla_len = nla_len,
> + .nla_type = INET_DIAG_CONG
> + );
> + memcpy(RTA_DATA(nla), "123", 4);
... is the same as this magic constant 4, which is the same
as sizeof("123"), ...
> + rc = sendto(fd, nlh, msg_len, MSG_DONTWAIT, NULL, 0);
> +
> + printf("sendto(%d, {{len=%u, type=SOCK_DIAG_BY_FAMILY"
> + ", flags=NLM_F_DUMP, seq=0, pid=0}, {idiag_family=AF_INET"
> + ", idiag_state=TCP_LISTEN, idiag_timer=0, idiag_retrans=0"
> + ", id={idiag_sport=htons(0), idiag_dport=htons(0)"
> + ", inet_pton(AF_INET, \"%s\", &idiag_src)"
> + ", inet_pton(AF_INET, \"%s\", &idiag_dst)"
> + ", idiag_if=0, idiag_cookie=[0, 0]}, idiag_expires=0"
> + ", idiag_rqueue=0, idiag_wqueue=0, idiag_uid=0"
> + ", idiag_inode=0}, {{nla_len=%u, nla_type=INET_DIAG_CONG}"
> + ", \"123\"}}, %u, MSG_DONTWAIT, NULL, 0) = %s\n",
... and this "123" must match the "123" used above.
In cases like this I suggest creating a static constant string and use it
instead, e.g.
static const char str[] = "123";
...
nla_len = NLA_HDRLEN + sizeof(str);
...
memcpy(RTA_DATA(nla), str, sizeof(str));
...
", \"%s\"}}, %u, MSG_DONTWAIT, NULL, 0) = %s\n", ..., str, ...);
[...]
> + /* short read of mark */
> + nla_len = NLA_HDRLEN + sizeof(*mark);
> + msg_len = NLMSG_SPACE(hdrlen) + nla_len;
> + nlh = nlh0 - (nla_len - 1);
> + init_inet_diag_msg(nlh, msg_len, address);
> +
> + nla = NLMSG_ATTR(nlh, hdrlen);
> + *nla = (struct nlattr) {
> + .nla_len = nla_len,
> + .nla_type = INET_DIAG_MARK
> + };
This is unaligned access.
> +
> + rc = sendto(fd, nlh, msg_len, MSG_DONTWAIT, NULL, 0);
> +
> + printf("sendto(%d, {{len=%u, type=SOCK_DIAG_BY_FAMILY"
> + ", flags=NLM_F_DUMP, seq=0, pid=0}, {idiag_family=AF_INET"
> + ", idiag_state=TCP_LISTEN, idiag_timer=0, idiag_retrans=0"
> + ", id={idiag_sport=htons(0), idiag_dport=htons(0)"
> + ", inet_pton(AF_INET, \"%s\", &idiag_src)"
> + ", inet_pton(AF_INET, \"%s\", &idiag_dst)"
> + ", idiag_if=0, idiag_cookie=[0, 0]}, idiag_expires=0"
> + ", idiag_rqueue=0, idiag_wqueue=0, idiag_uid=0"
> + ", idiag_inode=0}, {{nla_len=%u, nla_type=INET_DIAG_MARK}"
> + ", %p}}, %u, MSG_DONTWAIT, NULL, 0) = %s\n",
> + fd, msg_len, address, address, nla_len, RTA_DATA(nla),
> + msg_len, sprintrc(rc));
> +
> + /* mark */
> + nla_len = NLA_HDRLEN + sizeof(*mark);
> + msg_len = NLMSG_SPACE(hdrlen) + nla_len;
> + nlh = nlh0 - nla_len;
> + init_inet_diag_msg(nlh, msg_len, address);
> +
> + nla = NLMSG_ATTR(nlh, hdrlen);
> + *nla = (struct nlattr) {
> + .nla_len = nla_len,
> + .nla_type = INET_DIAG_MARK
> + };
There is no unaligned access here but I suggest to use the same
initialization method for all cases like this.
--
ldv
signature.asc
Description: PGP signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel
