[PATCH v1] Changing print_dev_t to print raw device number if -Xraw

[Dmitry V. Levin]

On Sat, Dec 08, 2018 at 02:17:14PM -0800, shankarapailoor wrote:
> Hi Dmitry,
> 
> Attached is a patch which changes print_dev_t to handle -Xraw and
> -Xverbose properly. I have added new tests fstat-Xraw, -Xabbrev, and
> -Xverbose respectively. The tests seem to pass fine for me, but let me
> know if there are any issues.

Thanks, this looks good, see my comments below.

> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -89,6 +89,9 @@ finit_module
>  flock
>  fork-f
>  fstat
> +fstat-Xraw
> +fstat-Xverbose
> +fstat-Xabbrev
>  fstat64
>  fstatat64
>  fstatfs

fstat is the name of syscall on 64-bit architectures,
new 32-bit architectures don't have fstat, just fstat64.

Would you mind adding the same -X tests as you did for fstat
also for fstat64?

> --- a/tests/btrfs.c
> +++ b/tests/btrfs.c
> @@ -183,10 +183,28 @@ sprint_xlat_(uint32_t val, const char *xlat)
>  
>  		return str;
>  	}
> -
>  	return xlat;
>  }

I don't think the removal of this empty line is relevant to the change.

> +static const char *
> +sprint_makedev(unsigned long long val)
> +{
> +	static char devid[256];
> +	int ret;
> +
> +	if (verbose_xlat)
> +		ret = snprintf(devid, sizeof(devid), "%#llx /* makedev(%#x, %#x) */", val, major(val), minor(val));
> +	else
> +		ret = snprintf(devid, sizeof(devid), "makedev(%#x, %#x)", major(val), minor(val));

These lines are too long, please wrap them.

> --- a/tests/xstatx.c
> +++ b/tests/xstatx.c
> @@ -121,6 +121,7 @@ typedef off_t libc_off_t;
>  #  define IS_STATX 0
>  # endif
>  
> +#if !XLAT_RAW /* Fixes -Wunused warning */
>  static void
>  print_ftype(const unsigned int mode)
>  {
> @@ -141,20 +142,46 @@ print_perms(const unsigned int mode)
>  {
>  	printf("%#o", mode & ~S_IFMT);
>  }
> +#endif
> +
> +static void
> +print_st_mode(const unsigned int mode)
> +{
> +#if XLAT_RAW
> +	printf("%#o", mode);
> +#elif XLAT_VERBOSE
> +	printf("%#o /* ", mode);
> +	print_ftype(mode);
> +	printf("|");
> +	print_perms(mode);
> +	printf(" */");
> +#else
> +	print_ftype(mode);
> +	printf("|");
> +	print_perms(mode);
> +#endif
> +}
>  
>  # if !IS_STATX
>  
>  static void
>  print_stat(const STRUCT_STAT *st)
>  {
> +#if XLAT_RAW
> +	printf("{st_dev=%#lx", st->st_dev);
> +#elif XLAT_VERBOSE
> +	printf("{st_dev=%#lx /* makedev(%#x, %#x) */",
> +		st->st_dev,
> +		(unsigned int) major(zero_extend_signed_to_ull(st->st_dev)),
> +		(unsigned int) minor(zero_extend_signed_to_ull(st->st_dev)));
> +#else
>  	printf("{st_dev=makedev(%#x, %#x)",
> -	       (unsigned int) major(zero_extend_signed_to_ull(st->st_dev)),
> -	       (unsigned int) minor(zero_extend_signed_to_ull(st->st_dev)));
> +		(unsigned int) major(zero_extend_signed_to_ull(st->st_dev)),
> +		(unsigned int) minor(zero_extend_signed_to_ull(st->st_dev)));
> +#endif

Why don't you use the same sprint_makedev approach in this test, too?


-- 
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/20181209/2fcca9ce/attachment.bin>