diff mbox series

nfs4_setfacl: add a specific option for indexes

Message ID 20220815083908.65720-1-plambri@redhat.com (mailing list archive)
State New, archived
Headers show
Series nfs4_setfacl: add a specific option for indexes | expand

Commit Message

Pierguido Lambri Aug. 15, 2022, 8:39 a.m. UTC
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 <plambri@redhat.com>
---
 nfs4_setfacl/nfs4_setfacl.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

Comments

Steve Dickson Aug. 24, 2022, 8:10 p.m. UTC | #1
Hello,

I'll go ahead and que this patch.. but there needs to be
an manpage update for me to commit to it...

steved.
On 8/15/22 4:39 AM, 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 <plambri@redhat.com>
> ---
>   nfs4_setfacl/nfs4_setfacl.c | 37 +++++++++++++++++--------------------
>   1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/nfs4_setfacl/nfs4_setfacl.c b/nfs4_setfacl/nfs4_setfacl.c
> index d0485ad..c3bdf56 100644
> --- a/nfs4_setfacl/nfs4_setfacl.c
> +++ b/nfs4_setfacl/nfs4_setfacl.c
> @@ -148,7 +148,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;
> @@ -158,21 +158,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;
> @@ -184,9 +177,6 @@ int main(int argc, char **argv)
>   				break;
>   
>   			case 'x':
> -				ace_index = strtoul_reals(optarg, 10);
> -				if(ace_index == ULONG_MAX)
> -					mod_string = optarg;
>   				goto remove;
>   			case 'X':
>   				spec_file = optarg;
> @@ -248,6 +238,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;
> @@ -283,6 +276,9 @@ int main(int argc, char **argv)
>   	if (action == NO_ACTION) {
>   		fprintf(stderr, "No action specified.\n");
>   		goto out;
> +	} 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 < 1) {
>   		fprintf(stderr, "No path(s) specified.\n");
>   		goto out;
> @@ -548,9 +544,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"
Pierguido Lambri Aug. 25, 2022, 7:43 a.m. UTC | #2
On Wed, Aug 24, 2022 at 04:10:40PM -0400, Steve Dickson wrote:
> Hello,
> 
> I'll go ahead and que this patch.. but there needs to be
> an manpage update for me to commit to it...

Thanks Steve.
I wasn't sure this would be accepted, I'll send another patch for the man
page.

Pier
Steve Dickson Sept. 13, 2022, 3:59 p.m. UTC | #3
On 8/25/22 3:43 AM, Pierguido Lambri wrote:
> On Wed, Aug 24, 2022 at 04:10:40PM -0400, Steve Dickson wrote:
>> Hello,
>>
>> I'll go ahead and que this patch.. but there needs to be
>> an manpage update for me to commit to it...
> 
> Thanks Steve.
> I wasn't sure this would be accepted, I'll send another patch for the man
> page.
FYI... I'm going to wait for the man page before I
do the commit.

steved.

> 
> Pier
>
Pierguido Lambri Sept. 14, 2022, 5:33 p.m. UTC | #4
On Tue, Sep 13, 2022 at 11:59:31AM -0400, Steve Dickson wrote:
> 
> FYI... I'm going to wait for the man page before I
> do the commit.
> 
Sorry for the delay, I just sent a new version with a couple of fixes and
the man page.

Thanks,

Pier
diff mbox series

Patch

diff --git a/nfs4_setfacl/nfs4_setfacl.c b/nfs4_setfacl/nfs4_setfacl.c
index d0485ad..c3bdf56 100644
--- a/nfs4_setfacl/nfs4_setfacl.c
+++ b/nfs4_setfacl/nfs4_setfacl.c
@@ -148,7 +148,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;
@@ -158,21 +158,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;
@@ -184,9 +177,6 @@  int main(int argc, char **argv)
 				break;
 
 			case 'x':
-				ace_index = strtoul_reals(optarg, 10);
-				if(ace_index == ULONG_MAX)
-					mod_string = optarg;
 				goto remove;
 			case 'X':
 				spec_file = optarg;
@@ -248,6 +238,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;
@@ -283,6 +276,9 @@  int main(int argc, char **argv)
 	if (action == NO_ACTION) {
 		fprintf(stderr, "No action specified.\n");
 		goto out;
+	} 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 < 1) {
 		fprintf(stderr, "No path(s) specified.\n");
 		goto out;
@@ -548,9 +544,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"