* [PATCH] io/attr.c: Disallow specifying both -D and -R options for chattr command
@ 2020-07-23 5:27 Xiao Yang
2020-07-23 6:08 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Xiao Yang @ 2020-07-23 5:27 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs, Xiao Yang
-D and -R options are mutually exclusive actually but chattr command
doesn't check it so that always applies -D option when both of them
are specified. For example:
------------------------------------
# mkdir testdir
# mkdir testdir/tdir
# touch testdir/tfile
# xfs_io -c "chattr -D -R +s" testdir
# xfs_io -c "lsattr -R" testdir
----s----------- testdir/tdir
---------------- testdir/tfile
----s----------- testdir
------------------------------------
Add a check to disallow the combination.
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
io/attr.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/io/attr.c b/io/attr.c
index 80e28514..f82a0881 100644
--- a/io/attr.c
+++ b/io/attr.c
@@ -320,6 +320,13 @@ chattr_f(
}
}
+ if (recurse_all && recurse_dir) {
+ fprintf(stderr, _("%s: -R and -D options are mutually exclusive\n"),
+ progname);
+ exitcode = 1;
+ return 0;
+ }
+
if (recurse_all || recurse_dir) {
nftw(name, chattr_callback,
100, FTW_PHYS | FTW_MOUNT | FTW_DEPTH);
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] io/attr.c: Disallow specifying both -D and -R options for chattr command
2020-07-23 5:27 [PATCH] io/attr.c: Disallow specifying both -D and -R options for chattr command Xiao Yang
@ 2020-07-23 6:08 ` Christoph Hellwig
2020-07-23 6:39 ` Xiao Yang
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-07-23 6:08 UTC (permalink / raw)
To: Xiao Yang; +Cc: sandeen, darrick.wong, linux-xfs
On Thu, Jul 23, 2020 at 01:27:23PM +0800, Xiao Yang wrote:
> -D and -R options are mutually exclusive actually but chattr command
> doesn't check it so that always applies -D option when both of them
> are specified. For example:
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] io/attr.c: Disallow specifying both -D and -R options for chattr command
2020-07-23 6:08 ` Christoph Hellwig
@ 2020-07-23 6:39 ` Xiao Yang
2020-07-23 22:10 ` Dave Chinner
2020-07-23 22:15 ` Eric Sandeen
0 siblings, 2 replies; 6+ messages in thread
From: Xiao Yang @ 2020-07-23 6:39 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: sandeen, darrick.wong, linux-xfs
On 2020/7/23 14:08, Christoph Hellwig wrote:
> On Thu, Jul 23, 2020 at 01:27:23PM +0800, Xiao Yang wrote:
>> -D and -R options are mutually exclusive actually but chattr command
>> doesn't check it so that always applies -D option when both of them
>> are specified. For example:
> Looks good,
>
> Reviewed-by: Christoph Hellwig<hch@lst.de>
Hi,
Ah, I have a question after sending the patch:
Other commands(e.g. cowextsize) including the same options seem to avoid
the issue by accepting the last option, as below:
--------------------------------------------------------
io/cowextsize.c
141 while ((c = getopt(argc, argv, "DR")) != EOF) {
142 switch (c) {
143 case 'D':
144 recurse_all = 0;
145 recurse_dir = 1;
146 break;
147 case 'R':
148 recurse_all = 1;
149 recurse_dir = 0;
150 break;
Test:
# xfs_io -c "cowextsize -D -R" testdir
[0] testdir/tdir
[0] testdir/tfile
[0] testdir
[root@Fedora-31 ~]# xfs_io -c "cowextsize -R -D" testdir
[0] testdir/tdir
[0] testdir
--------------------------------------------------------
Perhaps, we should use the same solution. (not sure) :-)
Thanks,
Xiao Yang
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] io/attr.c: Disallow specifying both -D and -R options for chattr command
2020-07-23 6:39 ` Xiao Yang
@ 2020-07-23 22:10 ` Dave Chinner
2020-07-23 22:15 ` Eric Sandeen
1 sibling, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2020-07-23 22:10 UTC (permalink / raw)
To: Xiao Yang; +Cc: Christoph Hellwig, sandeen, darrick.wong, linux-xfs
On Thu, Jul 23, 2020 at 02:39:06PM +0800, Xiao Yang wrote:
> On 2020/7/23 14:08, Christoph Hellwig wrote:
> > On Thu, Jul 23, 2020 at 01:27:23PM +0800, Xiao Yang wrote:
> > > -D and -R options are mutually exclusive actually but chattr command
> > > doesn't check it so that always applies -D option when both of them
> > > are specified. For example:
> > Looks good,
> >
> > Reviewed-by: Christoph Hellwig<hch@lst.de>
> Hi,
>
> Ah, I have a question after sending the patch:
> Other commands(e.g. cowextsize) including the same options seem to avoid the
> issue by accepting the last option, as below:
> --------------------------------------------------------
> io/cowextsize.c
> 141 while ((c = getopt(argc, argv, "DR")) != EOF) {
> 142 switch (c) {
> 143 case 'D':
> 144 recurse_all = 0;
> 145 recurse_dir = 1;
> 146 break;
> 147 case 'R':
> 148 recurse_all = 1;
> 149 recurse_dir = 0;
> 150 break;
>
> Test:
> # xfs_io -c "cowextsize -D -R" testdir
> [0] testdir/tdir
> [0] testdir/tfile
> [0] testdir
> [root@Fedora-31 ~]# xfs_io -c "cowextsize -R -D" testdir
> [0] testdir/tdir
> [0] testdir
> --------------------------------------------------------
>
> Perhaps, we should use the same solution. (not sure) :-)
They should all operate the same way and, IMO, the order of the
parameters on the command line should not change the behaviour of
the command. Hence I think erroring out is better than what the
cowextsize code does above.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] io/attr.c: Disallow specifying both -D and -R options for chattr command
2020-07-23 6:39 ` Xiao Yang
2020-07-23 22:10 ` Dave Chinner
@ 2020-07-23 22:15 ` Eric Sandeen
2020-07-24 1:09 ` Xiao Yang
1 sibling, 1 reply; 6+ messages in thread
From: Eric Sandeen @ 2020-07-23 22:15 UTC (permalink / raw)
To: Xiao Yang, Christoph Hellwig; +Cc: darrick.wong, linux-xfs
On 7/22/20 11:39 PM, Xiao Yang wrote:
> On 2020/7/23 14:08, Christoph Hellwig wrote:
>> On Thu, Jul 23, 2020 at 01:27:23PM +0800, Xiao Yang wrote:
>>> -D and -R options are mutually exclusive actually but chattr command
>>> doesn't check it so that always applies -D option when both of them
>>> are specified. For example:
>> Looks good,
>>
>> Reviewed-by: Christoph Hellwig<hch@lst.de>
> Hi,
>
> Ah, I have a question after sending the patch:
> Other commands(e.g. cowextsize) including the same options seem to avoid the issue by accepting the last option, as below:
> --------------------------------------------------------
> io/cowextsize.c
> 141 while ((c = getopt(argc, argv, "DR")) != EOF) {
> 142 switch (c) {
> 143 case 'D':
> 144 recurse_all = 0;
> 145 recurse_dir = 1;
> 146 break;
> 147 case 'R':
> 148 recurse_all = 1;
> 149 recurse_dir = 0;
> 150 break;
Yep, I meant to look at this but hadn't gotten to it yet. These should all
be consistent, and I tend to agree with Dave that explicitly conflicting
incompatible options and erroring out is better than silently accepting
the last one specified.
And indeed help specifies that they are exclusive:
cowextsize_cmd.args = _("[-D | -R] [cowextsize]");
It'd be great if you want to send a V2 that makes the behavior (and
documentation) of any/all commands that accept [-D | -R] consistent.
Thanks,
-Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] io/attr.c: Disallow specifying both -D and -R options for chattr command
2020-07-23 22:15 ` Eric Sandeen
@ 2020-07-24 1:09 ` Xiao Yang
0 siblings, 0 replies; 6+ messages in thread
From: Xiao Yang @ 2020-07-24 1:09 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, darrick.wong, linux-xfs
On 2020/7/24 6:15, Eric Sandeen wrote:
> On 7/22/20 11:39 PM, Xiao Yang wrote:
>> On 2020/7/23 14:08, Christoph Hellwig wrote:
>>> On Thu, Jul 23, 2020 at 01:27:23PM +0800, Xiao Yang wrote:
>>>> -D and -R options are mutually exclusive actually but chattr command
>>>> doesn't check it so that always applies -D option when both of them
>>>> are specified. For example:
>>> Looks good,
>>>
>>> Reviewed-by: Christoph Hellwig<hch@lst.de>
>> Hi,
>>
>> Ah, I have a question after sending the patch:
>> Other commands(e.g. cowextsize) including the same options seem to avoid the issue by accepting the last option, as below:
>> --------------------------------------------------------
>> io/cowextsize.c
>> 141 while ((c = getopt(argc, argv, "DR")) != EOF) {
>> 142 switch (c) {
>> 143 case 'D':
>> 144 recurse_all = 0;
>> 145 recurse_dir = 1;
>> 146 break;
>> 147 case 'R':
>> 148 recurse_all = 1;
>> 149 recurse_dir = 0;
>> 150 break;
> Yep, I meant to look at this but hadn't gotten to it yet. These should all
> be consistent, and I tend to agree with Dave that explicitly conflicting
> incompatible options and erroring out is better than silently accepting
> the last one specified.
>
> And indeed help specifies that they are exclusive:
>
> cowextsize_cmd.args = _("[-D | -R] [cowextsize]");
>
> It'd be great if you want to send a V2 that makes the behavior (and
> documentation) of any/all commands that accept [-D | -R] consistent.
Hi Eric, Dave
Thanks for your suggestions.
I will send v2 patch to make the behavior consistent.
Thanks,
Xiao Yang
> Thanks,
> -Eric
>
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-24 1:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23 5:27 [PATCH] io/attr.c: Disallow specifying both -D and -R options for chattr command Xiao Yang
2020-07-23 6:08 ` Christoph Hellwig
2020-07-23 6:39 ` Xiao Yang
2020-07-23 22:10 ` Dave Chinner
2020-07-23 22:15 ` Eric Sandeen
2020-07-24 1:09 ` Xiao Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).