Re: [PATCH 15/19] mkfs: don't treat files as though they are block devices

Jan Tulak
2016-04-13T15:43:37Z

[-- Attachment #1.1: Type: text/plain, Size: 2203 bytes --]

On Sat, Apr 9, 2016 at 6:12 AM, Eric Sandeen <sandeen@sandeen.net> wrote:

>
>
> On 4/8/16 9:58 AM, Jan Tulak wrote:
> > This one causes at least one interesting issue:
> >
> > #mkfs/mkfs.xfs
> > Error accessing specified device (null): Bad address
> > Usage: mkfs.xfs
> > ...
> >
> > because:
> >
> > check_device_type(dfile, &xi.disfile, !dsize, !dfile,
> > Nflag ? NULL : &xi.dcreat,
> force_overwrite, "d");
> >
> > so "dfile" can be NULL, but that function immediately tries to stat
> it.
> >
> >
> > ​A simple if NULL, then usage() should take care of this...
>
> Well, think hard about that. If dfile == NULL and bails with usage(), then
> what is the point of having !dfile as a function argument?
>
> Validation of having a target specified as either -dfile= or as the last
> argument
> should probably happen before the call to check_device_type.
>
>
But what about this?

case D_NAME:
xi.dname = getstr(value, &dopts, D_ NAME);

[...]

if (argc - optind > 1) {
[print error]
} else if (argc - optind == 1) {
dfile = xi.volname = getstr(argv[optind], &dopts, D_NAME);
} else
dfile = xi.dname;​

}

[
​... until the first occurence of dfile]

check_device_type(dfile, &xi.disfile, !dsize, !dfile,
Nflag ? NULL : &xi.dcreat, force_overwrite, "d");
​
​so if there is -d name= and nothing at the end, then dfile contains D_NAME
value. If there is a path at the end, then, then no matter about -d name,
we have dfile filled with something. So I can't find a way how to have
dfile == NULL without omitting both a path as the last argument, and
-dname...

The no_name (!dfile) argument is in check_device_type() for a single reason
- raising error on
if (*isfile && (no_size || no_name))

But !string is true only when string==NULL. For an empty string: !"" is 0.
So I don't see the reason for no_name argument at all - it seems to
confuse, and in current use can be replaced with test to NULL, or !name.

DTTO for xi.logname/xi.rtname.

​Cheers,
Jan​

--
Jan Tulak
jtulak@redhat.com / jan@tulak.me

[-- Attachment #1.2: Type: text/html, Size: 5920 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]