[PATCH] Add btrfs ioctl support.

[Dmitry V. Levin]

On Wed, May 04, 2016 at 08:07:13PM -0400, jeffm at suse.com wrote:
[...]
> +static void
> +btrfs_print_objectid(uint64_t objectid)
> +{
> +	const char *str = xlookup(btrfs_tree_objectids, objectid);

As we have both xlookup and xlookup64, you probably want to use xlookup64
at least in all 64-bit lookups.

> +static void
> +__print_key_value(struct tcb *tcp, const char *name, uint64_t value)
> +{
> +	if (value == UINT64_MAX)
> +		tprintf(", %s=UINT64_MAX", name);
> +	else if (value || !abbrev(tcp))
> +		tprintf(", %s=%" PRIu64, name, value);
> +}
> +#define print_key_value(tcp, key, name)					\
> +	__print_key_value((tcp), #name, (key)->name)

Please use a name that doesn't start with "__", e.g. print_key_value__.

> +	/* take a signed int */
> +	case BTRFS_IOC_BALANCE_CTL:
> +		printnum_int(tcp, arg, ", %d");

Please don't include comma into the format string, it won't work well
when the integer cannot be fetched.  Just split it:

		tprints(", ");
		printnum_int(tcp, arg, "%d");

> +	case BTRFS_IOC_DEV_REPLACE: { /* RW */
> +		struct btrfs_ioctl_dev_replace_args args;
> +
> +		if (entering(tcp))
> +			tprints(", ");

You might want to add here some code used in other places, e.g.:

		else if (syserror(tcp))
			break;
		else
			tprints(" => ");

> +		if (umove_or_printaddr(tcp, arg, &args))
> +			break;

otherwise an address will be printed out of context in case of syserror.

> +		if (entering(tcp)) {
> +			tprints("{cmd=");
> +			printxvals(args.cmd, "BTRFS_IOCTL_DEV_REPLACE_CMD_???",
> +				   btrfs_dev_replace_cmds, NULL);
> +			if (args.cmd == BTRFS_IOCTL_DEV_REPLACE_CMD_START) {
> +				const char *str;
> +				tprintf(", start={srcdevid=%" PRI__u64 ", "
> +				   "cont_reading_from_srcdev_mode=%" PRI__u64
> +				   ", srcdev_name=",
> +				   args.start.srcdevid,
> +				   args.start.cont_reading_from_srcdev_mode);
> +
> +				str = (const char*) args.start.srcdev_name;
> +				print_quoted_string(str,
> +						sizeof(args.start.srcdev_name),
> +						QUOTE_0_TERMINATED);
> +				tprints(", tgtdev_name=");
> +				str = (const char*) args.start.tgtdev_name;
> +				print_quoted_string(str,
> +						sizeof(args.start.tgtdev_name),
> +						QUOTE_0_TERMINATED);
> +				tprints("}");
> +
> +			}
> +			tprints("}");
> +			return 0;
> +		}
> +
> +		tprints(" => {result=");

but do not forget to remove " => " from this place.

> +		tprintf("nr_items=%" PRI__u64, args.nr_items);
> +		printflags64(btrfs_dev_stats_flags, args.flags,
> +			     "BTRFS_DEV_STATS_???");

A comma is missing in the output.

> +		if (entering(tcp)) {
> +			/* Use subvolume id of the containing root */
> +			if (args.treeid == 0)
> +				/* abuse of auxstr to retain state */
> +				tcp->auxstr = (void *)1;

Let's play safe and add
			else
				tcp->auxstr = NULL;

> +	/* take a signed int */
> +	case FICLONE:	/* W */
> +		printnum_int(tcp, arg, ", %d");
> +		break;

Please don't include comma into the format string, it won't work well
when the integer cannot be fetched.  Just split it:
		tprints(", ");
		printnum_int(tcp, arg, "%d");

> +	case FIDEDUPERANGE: { /* RW */
> +		struct file_dedupe_range args;
> +		uint64_t info_addr;
> +		uint16_t i;
> +
> +		if (entering(tcp))
> +			tprints(", ");
> +		else if (syserror(tcp))
> +			break;
> +		else
> +			tprints(" => ");
> +
> +		if (umove_or_printaddr(tcp, arg, &args))
> +			break;
> +
> +		if (entering(tcp)) {
> +			tprintf("{src_offset=%" PRIu64 ", "
> +				"src_length=%" PRIu64 ", "
> +				"dest_count=%hu, info=",
> +				(uint64_t)args.src_offset,
> +				(uint64_t)args.src_length,
> +				(uint16_t)args.dest_count);
> +		} else
> +			tprints("{info=");
> +
> +		if (abbrev(tcp)) {
> +			tprints("...}");
> +			break;
> +		}
> +
> +		tprints("[");
> +		info_addr = arg + offsetof(typeof(args), info);
> +		for (i = 0; i < args.dest_count; i++) {
> +			struct file_dedupe_range_info info;
> +			uint64_t addr = info_addr + sizeof(info) * i;
> +			if (i)
> +				tprints(", ");
> +
> +			if (umoven(tcp, addr, sizeof(info), &info)) {
> +				tprints("...");
> +				break;
> +			}
> +
> +			if (entering(tcp))
> +				tprintf("{dest_fd=%" PRIi64 ", "
> +					"dest_offset=%" PRIu64 "}",
> +					(int64_t)info.dest_fd,
> +					(uint64_t)info.dest_offset);
> +			else {
> +				tprintf("{bytes_deduped=%" PRIu64 ", "
> +					"status=%d}",
> +					(uint64_t)info.bytes_deduped,
> +					info.status);
> +			}
> +
> +		}
> +
> +		tprints("]}");
> +		break;

How is it going to parse anything on exiting after this break statement?

> +	}
> +	default:
> +		return RVAL_DECODED;
> +	};
> +	return ret | RVAL_DECODED | 1;
> +}


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160513/97f3840b/attachment.bin>