2022-09-14 17:47:33

by Pierguido Lambri

[permalink] [raw]
Subject: [PATCH v2 1/2] nfs4_setfacl: add a specific option for indexes

nfs4_setfacl had the possibility to use an optional index
to add/remove an ACL entry.
This was causing some confusion as numeric files could be interpreted
as indexes.
This change adds an extra command line option '-i' to specifically
handle the indexes.
The index can be used only with certain operations (add and remove).
The new syntax, when using indexes, would be:

~]# nfs4_setfacl -i 3 -a A::101:rxtncy file123

Signed-off-by: Pierguido Lambri <[email protected]>
---
nfs4_setfacl/nfs4_setfacl.c | 60 +++++++++++++++++++++++--------------
1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/nfs4_setfacl/nfs4_setfacl.c b/nfs4_setfacl/nfs4_setfacl.c
index e581608..d10e073 100644
--- a/nfs4_setfacl/nfs4_setfacl.c
+++ b/nfs4_setfacl/nfs4_setfacl.c
@@ -143,7 +143,7 @@ int main(int argc, char **argv)
int opt, err = 1;
int numpaths = 0, curpath = 0;
char *tmp, **paths = NULL, *path = NULL, *spec_file = NULL;
- FILE *s_fp = NULL;
+ FILE *s_fp, *fd = NULL;

if (!strcmp(basename(argv[0]), "nfs4_editfacl")) {
action = EDIT_ACTION;
@@ -155,7 +155,7 @@ int main(int argc, char **argv)
return err;
}

- while ((opt = getopt_long(argc, argv, "-:a:A:s:S:x:X:m:ethvHRPL", long_options, NULL)) != -1) {
+ while ((opt = getopt_long(argc, argv, "-:a:A:i:s:S:x::X:m:ethvHRPL", long_options, NULL)) != -1) {
switch (opt) {
case 'a':
mod_string = optarg;
@@ -165,21 +165,14 @@ int main(int argc, char **argv)
add:
assert_wu_wei(action);
action = INSERT_ACTION;
-
- /* run along if no more args (defaults to ace_index 1 == prepend) */
- if (optind == argc)
- break;
- ace_index = strtoul_reals(argv[optind++], 10);
- if (ace_index == ULONG_MAX) {
- /* oops it wasn't an ace_index; reset */
- optind--;
- ace_index = -1;
- } else if (ace_index == 0) {
- fprintf(stderr, "Sorry, valid indices start at '1'.\n");
- goto out;
+ break;
+ case 'i':
+ ace_index = strtoul_reals(optarg, 10);
+ if (ace_index == 0) {
+ fprintf(stderr, "Sorry, valid indices start at '1'.\n");
+ goto out;
}
break;
-
case 's':
mod_string = optarg;
goto set;
@@ -191,9 +184,14 @@ int main(int argc, char **argv)
break;

case 'x':
- ace_index = strtoul_reals(optarg, 10);
- if(ace_index == ULONG_MAX)
- mod_string = optarg;
+ /* make sure we handle the argument even if
+ * it doesn't immediately follow the option
+ */
+ if (optarg == NULL && optind < argc && argv[optind][0] != '-')
+ {
+ optarg = argv[optind++];
+ }
+ mod_string = optarg;
goto remove;
case 'X':
spec_file = optarg;
@@ -255,6 +253,9 @@ int main(int argc, char **argv)
case 'A':
fprintf(stderr, "Sorry, -a requires an 'acl_spec', whilst -A requires a 'spec_file'.\n");
goto out;
+ case 'i':
+ fprintf(stderr, "Sorry, -i requires an index (numerical)\n");
+ goto out;
case 's':
fprintf(stderr, "Sorry, -s requires an 'acl_spec'.\n");
goto out;
@@ -297,7 +298,21 @@ int main(int argc, char **argv)
if (action == NO_ACTION) {
fprintf(stderr, "No action specified.\n");
goto out;
- } else if (numpaths < 1) {
+ } else if (action != INSERT_ACTION && action != REMOVE_ACTION && ace_index >= 0) {
+ fprintf(stderr, "Index can be used only with add or remove.\n");
+ goto out;
+ } else if (numpaths <= 0 && ace_index >= 0 && mod_string)
+ {
+ /* Make sure the argument is a file */
+ if (!(fd = fopen(mod_string, "r"))) {
+ fprintf(stderr, "No path(s) specified.\n");
+ goto out;
+ } else
+ fclose(fd);
+ paths = malloc(sizeof(char *) * (argc - optind + 1));
+ paths[numpaths++] = mod_string;
+ } else if (numpaths < 1)
+ {
fprintf(stderr, "No path(s) specified.\n");
goto out;
}
@@ -609,9 +624,10 @@ static void __usage(const char *name, int is_ef)
"%s %s -- manipulate NFSv4 file/directory access control lists\n"
"Usage: %s [OPTIONS] COMMAND file ...\n"
" .. where COMMAND is one of:\n"
- " -a acl_spec [index] add ACL entries in acl_spec at index (DEFAULT: 1)\n"
- " -A file [index] read ACL entries to add from file\n"
- " -x acl_spec | index remove ACL entries or entry-at-index from ACL\n"
+ " -a acl_spec add ACL entries in acl_spec at defaul index (DEFAULT: 1)\n"
+ " -A file read ACL entries to add from file\n"
+ " -i index use the entry-at-index from ACL (only for add and remove)\n"
+ " -x acl_speci remove ACL entries\n"
" -X file read ACL entries to remove from file\n"
" -s acl_spec set ACL to acl_spec (replaces existing ACL)\n"
" -S file read ACL entries to set from file\n"
--
2.37.3


2022-09-14 18:00:43

by Pierguido Lambri

[permalink] [raw]
Subject: [PATCH v2 2/2] nfs4_setfacl: update man page about new option index

The man page now reflects the newly added syntax to handle indexes.

Signed-off-by: Pierguido Lambri <[email protected]>
---
man/man1/nfs4_setfacl.1 | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/man/man1/nfs4_setfacl.1 b/man/man1/nfs4_setfacl.1
index 47ab517..61699ae 100644
--- a/man/man1/nfs4_setfacl.1
+++ b/man/man1/nfs4_setfacl.1
@@ -30,27 +30,21 @@ Refer to the
manpage for information about NFSv4 ACL terminology and syntax.
.SS COMMANDS
.TP
-.BR "-a " "\fIacl_spec\fP [\fIindex\fP]"
+.BR "-a " "\fIacl_spec\fP"
.RI "add the ACEs from " acl_spec " to " file "'s ACL."
-ACEs are inserted starting at the
-.IR index th
-position (DEFAULT: 1) of
+ACEs are inserted starting at the default position 1 of
.IR file "'s ACL."
.\".ns
.TP
-.BR "-A " "\fIacl_file\fP [\fIindex\fP]"
+.BR "-A " "\fIacl_file\fP "
.RI "add the ACEs from the acl_spec in " acl_file " to " file "'s ACL."
-ACEs are inserted starting at the
-.IR index th
-position (DEFAULT: 1) of
+ACEs are inserted starting at the default position 1 of
.IR file "'s ACL."
.TP
-.BI "-x " "acl_spec \fR|\fP index"
+.BI "-x " "acl_spec \fR"
delete ACEs matched from
.I acl_spec
-- or delete the
-.IR index th
-ACE - from
+from
.IR file 's
ACL. Note that the ordering of the ACEs in
.I acl_spec
@@ -61,6 +55,10 @@ delete ACEs matched from the acl_spec in
.IR acl_file " from " file "'s ACL."
Note that the ordering of the ACEs in the acl_spec does not matter.
.TP
+.BI "-i " "\fIindex\fP"
+.RI "ACEs are inserted or deleted starting at the " index "th position (DEFAULT: 1) of file's ACL.
+It can be used only with the add or delete action.
+.TP
.BI "-s " acl_spec
.RI "set " file "'s ACL to " acl_spec .
.TP
@@ -189,6 +187,10 @@ add the same ACE as above, but using aliases:
.br
$ nfs4_setfacl -a A::[email protected]:RX foo
.IP - 2
+add the same ACE as above, at index 2:
+.br
+ $ nfs4_setfacl -i 2 -a A::[email protected]:RX foo
+.IP - 2
edit existing ACL in a text editor and set modified ACL on clean save/exit:
.br
$ nfs4_setfacl -e foo
--
2.37.3

2022-09-20 18:51:12

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] nfs4_setfacl: add a specific option for indexes



On 9/14/22 1:31 PM, Pierguido Lambri wrote:
> nfs4_setfacl had the possibility to use an optional index
> to add/remove an ACL entry.
> This was causing some confusion as numeric files could be interpreted
> as indexes.
> This change adds an extra command line option '-i' to specifically
> handle the indexes.
> The index can be used only with certain operations (add and remove).
> The new syntax, when using indexes, would be:
>
> ~]# nfs4_setfacl -i 3 -a A::101:rxtncy file123
>
> Signed-off-by: Pierguido Lambri <[email protected]>
Committed (tag: nfs4-acl-tools-0.4.1-rc3)

steved.
> ---
> nfs4_setfacl/nfs4_setfacl.c | 60 +++++++++++++++++++++++--------------
> 1 file changed, 38 insertions(+), 22 deletions(-)
>
> diff --git a/nfs4_setfacl/nfs4_setfacl.c b/nfs4_setfacl/nfs4_setfacl.c
> index e581608..d10e073 100644
> --- a/nfs4_setfacl/nfs4_setfacl.c
> +++ b/nfs4_setfacl/nfs4_setfacl.c
> @@ -143,7 +143,7 @@ int main(int argc, char **argv)
> int opt, err = 1;
> int numpaths = 0, curpath = 0;
> char *tmp, **paths = NULL, *path = NULL, *spec_file = NULL;
> - FILE *s_fp = NULL;
> + FILE *s_fp, *fd = NULL;
>
> if (!strcmp(basename(argv[0]), "nfs4_editfacl")) {
> action = EDIT_ACTION;
> @@ -155,7 +155,7 @@ int main(int argc, char **argv)
> return err;
> }
>
> - while ((opt = getopt_long(argc, argv, "-:a:A:s:S:x:X:m:ethvHRPL", long_options, NULL)) != -1) {
> + while ((opt = getopt_long(argc, argv, "-:a:A:i:s:S:x::X:m:ethvHRPL", long_options, NULL)) != -1) {
> switch (opt) {
> case 'a':
> mod_string = optarg;
> @@ -165,21 +165,14 @@ int main(int argc, char **argv)
> add:
> assert_wu_wei(action);
> action = INSERT_ACTION;
> -
> - /* run along if no more args (defaults to ace_index 1 == prepend) */
> - if (optind == argc)
> - break;
> - ace_index = strtoul_reals(argv[optind++], 10);
> - if (ace_index == ULONG_MAX) {
> - /* oops it wasn't an ace_index; reset */
> - optind--;
> - ace_index = -1;
> - } else if (ace_index == 0) {
> - fprintf(stderr, "Sorry, valid indices start at '1'.\n");
> - goto out;
> + break;
> + case 'i':
> + ace_index = strtoul_reals(optarg, 10);
> + if (ace_index == 0) {
> + fprintf(stderr, "Sorry, valid indices start at '1'.\n");
> + goto out;
> }
> break;
> -
> case 's':
> mod_string = optarg;
> goto set;
> @@ -191,9 +184,14 @@ int main(int argc, char **argv)
> break;
>
> case 'x':
> - ace_index = strtoul_reals(optarg, 10);
> - if(ace_index == ULONG_MAX)
> - mod_string = optarg;
> + /* make sure we handle the argument even if
> + * it doesn't immediately follow the option
> + */
> + if (optarg == NULL && optind < argc && argv[optind][0] != '-')
> + {
> + optarg = argv[optind++];
> + }
> + mod_string = optarg;
> goto remove;
> case 'X':
> spec_file = optarg;
> @@ -255,6 +253,9 @@ int main(int argc, char **argv)
> case 'A':
> fprintf(stderr, "Sorry, -a requires an 'acl_spec', whilst -A requires a 'spec_file'.\n");
> goto out;
> + case 'i':
> + fprintf(stderr, "Sorry, -i requires an index (numerical)\n");
> + goto out;
> case 's':
> fprintf(stderr, "Sorry, -s requires an 'acl_spec'.\n");
> goto out;
> @@ -297,7 +298,21 @@ int main(int argc, char **argv)
> if (action == NO_ACTION) {
> fprintf(stderr, "No action specified.\n");
> goto out;
> - } else if (numpaths < 1) {
> + } else if (action != INSERT_ACTION && action != REMOVE_ACTION && ace_index >= 0) {
> + fprintf(stderr, "Index can be used only with add or remove.\n");
> + goto out;
> + } else if (numpaths <= 0 && ace_index >= 0 && mod_string)
> + {
> + /* Make sure the argument is a file */
> + if (!(fd = fopen(mod_string, "r"))) {
> + fprintf(stderr, "No path(s) specified.\n");
> + goto out;
> + } else
> + fclose(fd);
> + paths = malloc(sizeof(char *) * (argc - optind + 1));
> + paths[numpaths++] = mod_string;
> + } else if (numpaths < 1)
> + {
> fprintf(stderr, "No path(s) specified.\n");
> goto out;
> }
> @@ -609,9 +624,10 @@ static void __usage(const char *name, int is_ef)
> "%s %s -- manipulate NFSv4 file/directory access control lists\n"
> "Usage: %s [OPTIONS] COMMAND file ...\n"
> " .. where COMMAND is one of:\n"
> - " -a acl_spec [index] add ACL entries in acl_spec at index (DEFAULT: 1)\n"
> - " -A file [index] read ACL entries to add from file\n"
> - " -x acl_spec | index remove ACL entries or entry-at-index from ACL\n"
> + " -a acl_spec add ACL entries in acl_spec at defaul index (DEFAULT: 1)\n"
> + " -A file read ACL entries to add from file\n"
> + " -i index use the entry-at-index from ACL (only for add and remove)\n"
> + " -x acl_speci remove ACL entries\n"
> " -X file read ACL entries to remove from file\n"
> " -s acl_spec set ACL to acl_spec (replaces existing ACL)\n"
> " -S file read ACL entries to set from file\n"

2022-09-20 18:51:12

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] nfs4_setfacl: update man page about new option index



On 9/14/22 1:31 PM, Pierguido Lambri wrote:
> The man page now reflects the newly added syntax to handle indexes.
>
> Signed-off-by: Pierguido Lambri <[email protected]>
Committed (tag: nfs4-acl-tools-0.4.1-rc3)

steved.
> ---
> man/man1/nfs4_setfacl.1 | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/man/man1/nfs4_setfacl.1 b/man/man1/nfs4_setfacl.1
> index 47ab517..61699ae 100644
> --- a/man/man1/nfs4_setfacl.1
> +++ b/man/man1/nfs4_setfacl.1
> @@ -30,27 +30,21 @@ Refer to the
> manpage for information about NFSv4 ACL terminology and syntax.
> .SS COMMANDS
> .TP
> -.BR "-a " "\fIacl_spec\fP [\fIindex\fP]"
> +.BR "-a " "\fIacl_spec\fP"
> .RI "add the ACEs from " acl_spec " to " file "'s ACL."
> -ACEs are inserted starting at the
> -.IR index th
> -position (DEFAULT: 1) of
> +ACEs are inserted starting at the default position 1 of
> .IR file "'s ACL."
> .\".ns
> .TP
> -.BR "-A " "\fIacl_file\fP [\fIindex\fP]"
> +.BR "-A " "\fIacl_file\fP "
> .RI "add the ACEs from the acl_spec in " acl_file " to " file "'s ACL."
> -ACEs are inserted starting at the
> -.IR index th
> -position (DEFAULT: 1) of
> +ACEs are inserted starting at the default position 1 of
> .IR file "'s ACL."
> .TP
> -.BI "-x " "acl_spec \fR|\fP index"
> +.BI "-x " "acl_spec \fR"
> delete ACEs matched from
> .I acl_spec
> -- or delete the
> -.IR index th
> -ACE - from
> +from
> .IR file 's
> ACL. Note that the ordering of the ACEs in
> .I acl_spec
> @@ -61,6 +55,10 @@ delete ACEs matched from the acl_spec in
> .IR acl_file " from " file "'s ACL."
> Note that the ordering of the ACEs in the acl_spec does not matter.
> .TP
> +.BI "-i " "\fIindex\fP"
> +.RI "ACEs are inserted or deleted starting at the " index "th position (DEFAULT: 1) of file's ACL.
> +It can be used only with the add or delete action.
> +.TP
> .BI "-s " acl_spec
> .RI "set " file "'s ACL to " acl_spec .
> .TP
> @@ -189,6 +187,10 @@ add the same ACE as above, but using aliases:
> .br
> $ nfs4_setfacl -a A::[email protected]:RX foo
> .IP - 2
> +add the same ACE as above, at index 2:
> +.br
> + $ nfs4_setfacl -i 2 -a A::[email protected]:RX foo
> +.IP - 2
> edit existing ACL in a text editor and set modified ACL on clean save/exit:
> .br
> $ nfs4_setfacl -e foo