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

[shankarapailoor]

Thanks for the comments. Attached is a v2 based on your feedback.
On Sat, Dec 8, 2018 at 3:24 PM Dmitry V. Levin <ldv at altlinux.org> wrote:
>
> 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
> --
> Strace-devel mailing list
> Strace-devel at lists.strace.io
> https://lists.strace.io/mailman/listinfo/strace-devel



-- 
Regards,
Shankara Pailoor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v2-0001-print_dev_t-print-raw-device-value-under-Xraw-ins.patch
Type: text/x-patch
Size: 13164 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20181208/ff887fe9/attachment.bin>