[PATCH v1] update quotactl to not print QCMD macro under Xraw

[Dmitry V. Levin]

Hi,

On Tue, Nov 27, 2018 at 08:06:41PM -0800, shankarapailoor wrote:
> Attached is a patch which makes it so we don't print the QCMD macro
> when running under -Xraw.

Thanks, see my comments below.

> Regards,
> Shankara Pailoor

> From bce487fbb29600f6ea6906f03941584f5968d079 Mon Sep 17 00:00:00 2001
> From: Shankara Pailoor <shankarapailoor at gmail.com>
> Date: Tue, 27 Nov 2018 20:00:19 -0800
> Subject: [PATCH v1] update quotactl to not print QCMD macro under -Xraw
> 
> ---
>  quota.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/quota.c b/quota.c
> index 383e4b17..be8219f9 100644
> --- a/quota.c
> +++ b/quota.c
> @@ -428,12 +428,27 @@ SYS_FUNC(quotactl)
>  	uint32_t id = tcp->u_arg[2];
>  
>  	if (entering(tcp)) {
> +		if (xlat_verbose(xlat_verbosity) != XLAT_STYLE_ABBREV)
> +			tprintf("%x", qcmd);
> +
> +		if (xlat_verbose(xlat_verbosity) == XLAT_STYLE_RAW)
> +			goto end;
> +
> +		if (xlat_verbose(xlat_verbosity) == XLAT_STYLE_VERBOSE)
> +			tprints(" /* ");
> +		
>  		tprints("QCMD(");
>  		printxval(quotacmds, cmd, "Q_???");
>  		tprints(", ");
>  		printxval(quotatypes, type, "???QUOTA");
> -		tprints("), ");
> -		printpath(tcp, tcp->u_arg[1]);
> +		tprints(")");
> +
> +		if (xlat_verbose(xlat_verbosity) == XLAT_STYLE_VERBOSE)
> +			tprints(" */");
> +
>  	}
> +end:
> +	tprints(", ");
> +	printpath(tcp, tcp->u_arg[1]);
>  	return decode_cmd_data(tcp, id, cmd, tcp->u_arg[3]);
>  }

I see three problems with this approach.

First, this parser is going to print tcp->u_arg[1] twice for such commands
as Q_GETQUOTA.  I suppose the test suite detects this.  Have you tried
to run the test suite?
I suggest to move the code that prints QCMD into a separate function,
this way you'd keep it simple and avoid mistakes.

Second, this parser is going to print something strange in "-X verbose"
mode because printxval() also handles xlat verbosity.
I suggest to replace printxval() calls with printxvals_ex() like it was
done recently in commit v4.25-26-g0933b3086.

Third, there are no tests for this change.
If you added a test, it would detect the oddness in -X verbose" output.
Please add some tests like those that were added recently by commits
v4.25-27-ga4fb2fbdd and v4.25-25-g956dc3ed3.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20181202/90d8bbfa/attachment.bin>