Re: [PATCH 02/19] mkfs: sanitise ftype parameter values.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 29, 2016 at 6:17 PM, Eric Sandeen <sandeen@xxxxxxxxxxx> wrote:


On 3/29/16 11:11 AM, Jan Tulak wrote:
> On Thu, Mar 24, 2016 at 5:33 PM, Eric Sandeen <sandeen@xxxxxxxxxxx <mailto:sandeen@xxxxxxxxxxx>>wrote:
>
>
>
>     On 3/24/16 6:15 AM, jtulak@xxxxxxxxxx <mailto:jtulak@xxxxxxxxxx> wrote:
>     > From: Dave Chinner <dchinner@xxxxxxxxxx <mailto:dchinner@xxxxxxxxxx>>
>     >
>     > Because passing "-n ftype=2" should fail.
>
>     but passing crc=1 ftype=1 shouldn't fail, should it?
>     Seems like it will here.
>
>
> ​From man page:
> When  CRCs are enabled via -m crc=1, the ftype functionality is always enabled. This feature can not be
> ​
> turned off for such filesystem configurations.​
>
>
> ​So I think it should not be possible to enter both crc and ftype at
> the same time - which is the current behaviour. It feels strange a
> bit to allow ftype=1 (which does nothing with crc=1), but fail on
> ftype=0​

My point is that -m crc=1 -d ftype=1 simply restates the defaults.
Why should that combination fail?

And -m crc=0 -d ftype=0 is also perfectly acceptable.

In fact, -m crc=1 -d ftype=0 is the only one of the 4 combinations
which is not ok, but AFAICT your patch fails the other 3 as well.

​I removed the entire crc check - there is another one later in the code, so this was 1) redundant, 2) wrong, because it depended on whether -m crc=0 was before or after -n ftype (if after, default crc value was used for the check).

However, it causes lots of conflicts on the following patches, so I will send it later, once there are more changes...

Cheers, and thanks for catching it.

Jan

--
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs

[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux