On Thu, Apr 21, 2016 at 02:04:33PM +0100, Dr. David Alan Gilbert wrote:
[...]
> --- a/configure.ac
> +++ b/configure.ac
> @@ -420,6 +420,9 @@ AC_CHECK_HEADERS([linux/input.h], [
>       AC_CHECK_MEMBERS([struct input_absinfo.resolution],,, [#include 
> <linux/input.h>])
>  ])
>  
> +AC_CHECK_HEADERS([linux/userfaultfd.h], [
> +])

This should rather go to the first (big) AC_CHECK_HEADERS list
in the file.

> --- a/ioctl.c
> +++ b/ioctl.c
> @@ -263,6 +263,8 @@ ioctl_decode(struct tcb *tcp)
>       case 'E':
>               return evdev_ioctl(tcp, code, arg);
>  #endif
> +     case 0xaa:
> +             return uffdio_ioctl(tcp, code, arg);

The tradition approach is to wrap this into
#ifdef HAVE_LINUX_USERFAULTFD_H / #endif
so that ...

> --- a/userfaultfd.c
> +++ b/userfaultfd.c
> @@ -27,9 +27,50 @@
>  
>  #include "defs.h"
>  #include <fcntl.h>
> +#include <linux/ioctl.h>
> +#ifdef HAVE_LINUX_USERFAULTFD_H
> +#include <linux/userfaultfd.h>
> +#endif
>  
>  #include "xlat/uffd_flags.h"
>  
> +
> +int
> +uffdio_ioctl(struct tcb *tcp, const unsigned int code, const long arg)
> +{
> +#ifdef HAVE_LINUX_USERFAULTFD_H

... the whole parser with all headers it needs could be wrapped
with a single ifdef.

> +     case UFFDIO_COPY: {
> +             struct uffdio_copy uc;
> +             if (!umove_or_printaddr(tcp, arg, &uc)) {
> +                     if (entering(tcp)) {
> +                             tprintf("{dst=%" PRIx64 ", src=%" PRIx64
> +                             ", len=%" PRIu64 ", mode=%" PRIu64 "}",
> +                             (uint64_t)uc.dst, (uint64_t)uc.src,
> +                             (uint64_t)uc.len, (uint64_t)uc.mode);

I suppose you can use recently added PRI__*64 instead of PRI*64,
this way you won't need explicit casts.

> +                             return 1;
> +                     } else {
> +                             /* copy also returns the number of bytes
> +                              * copied */
> +                             sprintf(auxbuf, "copy=%" PRId64,
> +                             (int64_t)uc.copy);
> +                             tcp->auxstr = auxbuf;
> +                             return RVAL_STR | 1;
> +                     }

The preferred method is to print uc.copy as a part of structure:

        if (entering(tcp)) {
                if (umove_or_printaddr(tcp, arg, &uc))
                        return RVAL_DECODED | 1;
                tprintf("{dst=%" PRI__x64 ", src=%" PRI__x64
                        ", len=%" PRI__u64 ", mode=%" PRI__u64
                        ", copy=",
                        uc.dst, uc.src, uc.len, uc.mode);
                        return 1;
        } else {
                if (!umove_or_printaddr(tcp, arg, &uc.copy))
                        tprintf("%" PRI__d64, uc.copy);
                tprints("}");
                return 1;
        }

> +             }
> +             break;

As (your edition of) this parser has already printed something at this
point, it has to return 1 + RVAL_something to let the caller know,
otherwise the address will be printed twice.  You can easily reproduce
this by invoking UFFDIO_COPY with NULL address.

> +     }
> +#endif
> +
> +     default:
> +             return 0;

As this parser in default case is not going to do anything on exiting,
it report this to the caller by returning RVAL_DECODED.


-- 
ldv

Attachment: pgpubOJOmSOGU.pgp
Description: PGP signature

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to