On Sat, May 20, 2017 at 07:42:19PM +0800, JingPiao Chen wrote:
> * defs.h (NLA_*): New enum.
> (nla_policy): New struct.
> (decode_nlattr): Adapt prototype.
> * netlink.c (decode_nlattr_data): New function.
> (decode_nlattr_with_data): Use decode_nlattr_data.
> * netlink_sock_diag.c (decode_inet_diag_req_compat)
> (decode_inet_diag_req_v2, decode_inet_diag_msg)
> (decode_netlink_diag_msg, (decode_packet_diag_msg)
> (decode_smc_diag_msg, decode_unix_diag_msg): Add
> policy_size, policy, fallback_parser and opaque_data arguments.
> All callers updated.
> 
> Co-authored-by: Fabien Siron <fabien.si...@epita.fr>
> ---
>  defs.h              |  40 ++++++++++++-
>  netlink.c           | 161 
> +++++++++++++++++++++++++++++++++++++++++++++++++---
>  netlink_sock_diag.c |  14 ++---
>  3 files changed, 198 insertions(+), 17 deletions(-)
> 
> diff --git a/defs.h b/defs.h
> index 7e3912d..1ebb89e 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -630,11 +630,47 @@ extern void
>  tprint_iov_upto(struct tcb *, kernel_ulong_t len, kernel_ulong_t addr,
>               enum iov_decode, kernel_ulong_t data_size);
>  
> +enum {
> +     NLA_UNSPEC,
> +     NLA_U8,
> +     NLA_U16,
> +     NLA_U32,
> +     NLA_U64,
> +     NLA_STRING,
> +     NLA_FLAG,
> +     NLA_MSECS,
> +     NLA_NESTED,
> +     NLA_NESTED_COMPAT,
> +     NLA_NUL_STRING,
> +     NLA_BINARY,
> +     NLA_S8,
> +     NLA_S16,
> +     NLA_S32,
> +     NLA_S64
> +};

I think this enum doesn't belong to defs.h; would you mind creating
a separate header file, e.g. nlattr.h, and placing all nlattr related
declaration there?

> +
> +struct nla_policy {
> +     uint16_t type;
> +     uint16_t len;
> +};
> +
>  extern void
>  decode_netlink(struct tcb *, int fd, kernel_ulong_t addr, kernel_ulong_t 
> len);
>  extern void
> -decode_nlattr(struct tcb *tcp, kernel_ulong_t addr, kernel_ulong_t len,
> -           int, const struct xlat *, const char *);
> +decode_nlattr(struct tcb *tcp,
> +           kernel_ulong_t addr,
> +           kernel_ulong_t len,
> +           int hdrlen,
> +           const struct xlat *table,
> +           const char *dflt,
> +           unsigned int policy_size,
> +           const struct nla_policy *policy,

It would be more logical if "policy_size" argument was specified after
"policy" argument.

> +           bool (*fallback_parser)(struct tcb *,
> +                                   kernel_ulong_t addr,
> +                                   kernel_ulong_t len,
> +                                   int nla_type,
> +                                   void *opaque_data),
> +           void *const opaque_data);
>  
>  extern void tprint_open_modes(unsigned int);
>  extern const char *sprint_open_modes(unsigned int);
> diff --git a/netlink.c b/netlink.c
> index 52f3806..cadfccb 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -78,9 +78,138 @@ print_nlattr(const struct nlattr *const nla,
>  }
>  
>  static void
> -decode_nlattr_with_data(struct tcb *tcp, kernel_ulong_t addr,
> -                     kernel_ulong_t len, const struct xlat *table,
> -                     const char *dflt)
> +decode_nlattr_data(struct tcb *tcp,
> +                kernel_ulong_t addr,
> +                kernel_ulong_t len,
> +                unsigned int nla_type,
> +                unsigned int policy_size,
> +                const struct nla_policy *policy,
> +                bool(*fallback_parser)(struct tcb *,
> +                                       kernel_ulong_t addr,
> +                                       kernel_ulong_t len,
> +                                       int nla_type,
> +                                       void *opaque_data),
> +                void *const opaque_data)
> +{
> +     if (!policy || nla_type >= policy_size) {
> +             printstrn(tcp, addr, len);
> +             return;
> +     }
> +
> +     switch (policy[nla_type].type) {
> +     case NLA_U8: {
> +             uint8_t num;
> +
> +             if (len < sizeof(num))
> +                     printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> +             else if (!umove_or_printaddr(tcp, addr, &num))
> +                     tprintf("%" PRIu8, num);
> +             break;
> +     }
> +     case NLA_U16: {
> +             uint16_t num;
> +
> +             if (len < sizeof(num))
> +                     printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> +             else if (!umove_or_printaddr(tcp, addr, &num))
> +                     tprintf("%" PRIu16, num);
> +             break;
> +     }
> +     case NLA_U32: {
> +             uint32_t num;
> +
> +             if (len < sizeof(num))
> +                     printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> +             else if (!umove_or_printaddr(tcp, addr, &num))
> +                     tprintf("%" PRIu32, num);
> +             break;
> +     }
> +     case NLA_MSECS:
> +     case NLA_U64: {
> +             uint64_t num;
> +
> +             if (len < sizeof(num))
> +                     printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> +             else if (!umove_or_printaddr(tcp, addr, &num))
> +                     tprintf("%" PRIu64, num);
> +             break;
> +     }
> +     case NLA_STRING: {
> +             uint16_t str_len = len;
> +
> +             if (policy[nla_type].len && policy[nla_type].len < len)
> +                     str_len = policy[nla_type].len;
> +             printstrn(tcp, addr, str_len);
> +             break;
> +     }
> +     case NLA_NUL_STRING:
> +             printstr(tcp, addr);
> +             break;
> +     case NLA_S8: {
> +             int8_t num;
> +
> +             if (len < sizeof(num))
> +                     printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> +             else if (!umove_or_printaddr(tcp, addr, &num))
> +                     tprintf("%" PRId8, num);
> +             break;
> +     }
> +     case NLA_S16: {
> +             int16_t num;
> +
> +             if (len < sizeof(num))
> +                     printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> +             else if (!umove_or_printaddr(tcp, addr, &num))
> +                     tprintf("%" PRId16, num);
> +             break;
> +     }
> +     case NLA_S32: {
> +             int32_t num;
> +
> +             if (len < sizeof(num))
> +                     printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> +             else if (!umove_or_printaddr(tcp, addr, &num))
> +                     tprintf("%" PRId32, num);
> +             break;
> +     }
> +     case NLA_S64: {
> +             int64_t num;
> +
> +             if (len < sizeof(num))
> +                     printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> +             else if (!umove_or_printaddr(tcp, addr, &num))
> +                     tprintf("%" PRId64, num);
> +             break;
> +     }

All these numeric cases look very similar, could you think of something
that would save us from these cut-n-paste lines?

> +     case NLA_NESTED:
> +     case NLA_NESTED_COMPAT:
> +     case NLA_BINARY:
> +             if (fallback_parser
> +                 && fallback_parser(tcp, addr, len, nla_type, opaque_data))
> +                     break;
> +             /* fall through when fallback_parser return false */
> +     case NLA_FLAG:
> +     case NLA_UNSPEC:
> +     default:
> +             printstrn(tcp, addr, len);
> +             break;
> +     }
> +}
> +
> +static void
> +decode_nlattr_with_data(struct tcb *tcp,
> +                     kernel_ulong_t addr,
> +                     kernel_ulong_t len,
> +                     const struct xlat *table,
> +                     const char *dflt,
> +                     unsigned int policy_size,
> +                     const struct nla_policy *policy,
> +                     bool (*fallback_parser)(struct tcb *,
> +                                             kernel_ulong_t addr,
> +                                             kernel_ulong_t len,
> +                                             int nla_type,
> +                                             void *opaque_data),
> +                     void *const opaque_data)
>  {
>       struct nlattr nlattr;
>  
> @@ -100,8 +229,10 @@ decode_nlattr_with_data(struct tcb *tcp, kernel_ulong_t 
> addr,
>               print_nlattr(&nlattr, table, dflt);
>               if (nlattr.nla_len > NLA_HDRLEN) {
>                       tprints(", ");
> -                     printstrn(tcp, addr + NLA_HDRLEN,
> -                               nlattr.nla_len - NLA_HDRLEN);
> +                     decode_nlattr_data(tcp, addr + NLA_HDRLEN,
> +                                        nlattr.nla_len - NLA_HDRLEN,
> +                                        nlattr.nla_type, policy_size, policy,
> +                                        fallback_parser, opaque_data);
>               }
>               tprints("}");
>  
> @@ -115,13 +246,27 @@ decode_nlattr_with_data(struct tcb *tcp, kernel_ulong_t 
> addr,
>  }
>  
>  void
> -decode_nlattr(struct tcb *tcp, kernel_ulong_t addr, kernel_ulong_t len,
> -           int hdrlen, const struct xlat *table, const char *dflt)
> +decode_nlattr(struct tcb *tcp,
> +           kernel_ulong_t addr,
> +           kernel_ulong_t len,
> +           int hdrlen,
> +           const struct xlat *table,
> +           const char *dflt,
> +           unsigned int policy_size,
> +           const struct nla_policy *policy,
> +           bool (*fallback_parser)(struct tcb *,
> +                                   kernel_ulong_t addr,
> +                                   kernel_ulong_t len,
> +                                   int nla_type,
> +                                   void *opaque_data),
> +           void *const opaque_data)
>  {
>       if (len > NLMSG_ALIGN(hdrlen)) {
>               tprints(", ");
>               decode_nlattr_with_data(tcp, addr + NLMSG_ALIGN(hdrlen),
> -                                     len - NLMSG_ALIGN(hdrlen), table, dflt);
> +                                     len - NLMSG_ALIGN(hdrlen),
> +                                     table, dflt, policy_size, policy,
> +                                     fallback_parser, opaque_data);
>       }
>  }
>  
> diff --git a/netlink_sock_diag.c b/netlink_sock_diag.c
> index 36c949b..dc8ff43 100644
> --- a/netlink_sock_diag.c
> +++ b/netlink_sock_diag.c
> @@ -143,7 +143,7 @@ decode_unix_diag_msg(struct tcb *const tcp,
>       tprints("}");
>  
>       decode_nlattr(tcp, addr, len, sizeof(msg), unix_diag_attrs,
> -                   "UNIX_DIAG_???");
> +                   "UNIX_DIAG_???", 0, NULL, NULL, NULL);
>  }
>  
>  static void
> @@ -226,7 +226,7 @@ decode_netlink_diag_msg(struct tcb *const tcp,
>       tprints("}");
>  
>       decode_nlattr(tcp, addr, len, sizeof(msg), netlink_diag_attrs,
> -                   "NETLINK_DIAG_???");
> +                   "NETLINK_DIAG_???", 0, NULL, NULL, NULL);
>  }
>  
>  static void
> @@ -291,7 +291,7 @@ decode_packet_diag_msg(struct tcb *const tcp,
>       tprints("}");
>  
>       decode_nlattr(tcp, addr, len, sizeof(msg), packet_diag_attrs,
> -                   "PACKET_DIAG_???");
> +                   "PACKET_DIAG_???", 0, NULL, NULL, NULL);
>  }
>  
>  static void
> @@ -349,7 +349,7 @@ decode_inet_diag_req_compat(struct tcb *const tcp,
>       tprints("}");
>  
>       decode_nlattr(tcp, addr, len, sizeof(req), inet_diag_req_attrs,
> -                   "INET_DIAG_REQ_???");
> +                   "INET_DIAG_REQ_???", 0, NULL, NULL, NULL);
>  }
>  
>  static void
> @@ -387,7 +387,7 @@ decode_inet_diag_req_v2(struct tcb *const tcp,
>       tprints("}");
>  
>       decode_nlattr(tcp, addr, len, sizeof(req), inet_diag_req_attrs,
> -                   "INET_DIAG_REQ_???");
> +                   "INET_DIAG_REQ_???", 0, NULL, NULL, NULL);
>  }
>  
>  static void
> @@ -447,7 +447,7 @@ decode_inet_diag_msg(struct tcb *const tcp,
>       tprints("}");
>  
>       decode_nlattr(tcp, addr, len, sizeof(msg), inet_diag_attrs,
> -                   "INET_DIAG_???");
> +                   "INET_DIAG_???", 0, NULL, NULL, NULL);
>  }
>  
>  #ifdef AF_SMC
> @@ -521,7 +521,7 @@ decode_smc_diag_msg(struct tcb *const tcp,
>       tprints("}");
>  
>       decode_nlattr(tcp, addr, len, sizeof(msg), smc_diag_attrs,
> -                   "SMC_DIAG_???");
> +                   "SMC_DIAG_???", 0, NULL, NULL, NULL);
>  }
>  #endif
>  

-- 
ldv

Attachment: 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

Reply via email to