[PATCH] Implement decoding of IOCTL_VM_SOCKETS_GET_LOCAL_CID

Eugene Syromyatnikov evgsyr at gmail.com
Fri Jul 26 13:40:22 UTC 2024


Thank you for your patch.  Some notes below:

On Thu, Jul 25, 2024 at 3:09 PM Alyssa Ross <hi at alyssa.is> wrote:
> I don't think it's really possible to write a meaningful test for this
> — when we called ioctl(), we'd have no idea in advance what we'd
> expect it to return, so the bulk of the test would end up being an xlat
> implementation rather than anything useful.

Well, for decoders, testing them is basically verifying that their
output matches the expectations;  for xlats, that would be that flags
are not erroneously decoded as constants and vice versa, for example.
There's also an idea of checking against possible future values, to
force a decoder update (such as a constant list update or a new
command update) to update the corresponding test(s), to make such
updates more conscionable.
tests/ioctl_lirc.c (together with tests/ioctl_lirc-success.c is
probably an uncomplicated enough example that covers it.

> --- /dev/null
> +++ b/src/vsock_ioctl.c
> @@ -0,0 +1,40 @@
> +/*

Having a short description of the file's purpose would be nice here,
however trivial it is.

> +#include "xlat/vsock_cids.h"

Since this xlat is already used elsewhere, you don't actually need to
include it here (to avoid unnecessary double definition of the xlat,
as it is already defined by the virtue of inclusion in
src/sockaddr.c), but rather add "extern const struct xlat
vsock_cids[];" to src/defs.h, to have the xlat header file generated
accordingly and have the declaration available for use.

> +int
> +vsock_ioctl(struct tcb *const tcp, const unsigned int code,
> +           const kernel_ulong_t arg)
> +{
> +       unsigned int cid;
> +
> +       switch (code) {
> +       case IOCTL_VM_SOCKETS_GET_LOCAL_CID:
> +               if (entering(tcp)) {
> +                       tprint_arg_next();
> +                       return 0;
> +               }
> +
> +               if (umove_or_printaddr(tcp, arg, &cid))
> +                       break;
> +

tprint_indirect_begin() is missing here.

> +               printxval(vsock_cids, cid, NULL);

tprint_indirect_end() is missing here.

> +
> +               break;
> +       default:
> +               return RVAL_DECODED;
> +       }
> +       return RVAL_IOCTL_DECODED;
> +}

This change (being user-facing) is probably worth mentioning in the
NEWS file, something along the lines of "Implemented decoding of
IOCTL_VM_SOCKETS_GET_LOCAL_CID ioctl command".

-- 
Eugene Syromyatnikov
mailto:evgsyr at gmail.com
xmpp:esyr at jabber.{ru|org}


More information about the Strace-devel mailing list