Hi, On Thu, Feb 07, 2013 at 03:15:17PM +0000, Frediano Ziglio wrote: > I was trying to add support to Xen ioctl to strace. To distinguish > from other ioctl I used the entire code (not only type and number but > also size and direction) so I changed sys_ioctl in io.c to call a > function that check for the entire code and return true if it handled > the syscall. Is it fine to do it?
It is probably OK, at least I couldn't think up a better alternative yet.
> The ioctl of Xen are quite complicated to decode cause there are single
> ioctls that accept a structure that internally have a command field that
> specify the operation and an union with all data for operations,
> something like
>
> struct my_operation {
> int operation;
> union {
> struct operation1_data operation1;
> struct operation2_data operation2;
> struct operation3_data operation3;
> ...
> } u;
> };
>
> Just to complicate more some field in operationX_data can be for output,
> input or input/output. I found no way to store some informations in tcb
> structure. I'd like to print something when syscall enters (input
> parameters) and something when syscall leaves, somethings like
>
> ioctl(FD, IOCTL, {op=my_op,u={field_inout=5}}->{u={field_inout=3}}) = 0
>
> (->) to separate input and output. Is there a specific syntax to
> separate input and output?
This is uncharted territory, we had nothing like this before.
Anyway, a space before and after this delimiter wouldn't harm.
> +extern int ioctl_decode_exact(struct tcb *tcp);
A declaration would have to go to defs.h
> int
> sys_ioctl(struct tcb *tcp)
> {
> @@ -384,6 +386,12 @@ sys_ioctl(struct tcb *tcp)
> if (entering(tcp)) {
> printfd(tcp, tcp->u_arg[0]);
> tprints(", ");
> + }
> +
> + if (ioctl_decode_exact(tcp))
> + return 0;
> +
> + if (entering(tcp)) {
> iop = ioctl_lookup(tcp->u_arg[1]);
> if (iop) {
> tprints(iop->symbol);
I'd rather put this ioctl_decode_exact() call to both branches.
> +#include <xenctrl.h>
> +#include <xen/sys/evtchn.h>
> +#include <xen/sys/privcmd.h>
Availability of these headers has to be checked in configure.ac so that
HAVE_XENCTRL_H and other macros could be used here.
I'd prefer to put this complicated xen ioctl parser to separate xen.c
source file.
> +// hold information on status
> +// need flush if space is not enough
> +typedef struct {
> + char *dest, *dest_end;
> + char buffer[256];
> +} print_status;
> +
> +static inline void
> +status_init(print_status *status)
> +{
> + status->dest = status->buffer;
> + status->dest_end = status->buffer + sizeof(status->buffer);
> +}
> +
> +static void
> +assure_space(print_status *status, size_t needed)
> +{
> + /* assert(needed <= sizeof(status->buffer)); */
> + if (needed > status->dest_end - status->dest) {
> + tprints(status->buffer);
> + status->buffer[0] = 0;
> + status->dest = status->buffer;
> + }
> +}
> +
> +static inline void
> +status_flush(print_status *status)
> +{
> + assure_space(status, sizeof(status->buffer));
> +}
> +
> +static void
> +status_printf(print_status *status, const char *fmt, ...)
> +{
> + int l;
> + va_list ap;
> +
> + assure_space(status, 64);
> + va_start(ap, fmt);
> + l = vsprintf(status->dest, fmt, ap);
> + va_end(ap);
> + status->dest += l;
> +}
Why do we need additional buffering on top of tprintf/tprints?
Does the latter perform so badly?
--
ldv
pgpHUhOumLpL8.pgp
Description: PGP signature
------------------------------------------------------------------------------ Free Next-Gen Firewall Hardware Offer Buy your Sophos next-gen firewall before the end March 2013 and get the hardware for free! Learn more. http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel
