diff mbox series

[3/4] xfs_quota: don't exit on fs_table_insert_project_path failure

Message ID 1639167697-15392-4-git-send-email-sandeen@sandeen.net (mailing list archive)
State Accepted, archived
Headers show
Series xfsprogs: misc small fixes | expand

Commit Message

Eric Sandeen Dec. 10, 2021, 8:21 p.m. UTC 
From: Eric Sandeen <sandeen@redhat.com>

If "project -p" fails in fs_table_insert_project_path, it
calls exit() today which is quite unfriendly. Return an error
and return to the command prompt as expected.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 libfrog/paths.c | 7 +++----
 libfrog/paths.h | 2 +-
 quota/project.c | 4 +++-
 3 files changed, 7 insertions(+), 6 deletions(-)

Comments

Darrick J. Wong Dec. 11, 2021, 12:21 a.m. UTC | #1 
On Fri, Dec 10, 2021 at 02:21:36PM -0600, Eric Sandeen wrote:
> From: Eric Sandeen <sandeen@redhat.com>
> 
> If "project -p" fails in fs_table_insert_project_path, it
> calls exit() today which is quite unfriendly. Return an error
> and return to the command prompt as expected.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>  libfrog/paths.c | 7 +++----
>  libfrog/paths.h | 2 +-
>  quota/project.c | 4 +++-
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/libfrog/paths.c b/libfrog/paths.c
> index d679376..6c0fee2 100644
> --- a/libfrog/paths.c
> +++ b/libfrog/paths.c
> @@ -546,7 +546,7 @@ out_error:
>  		progname, strerror(error));
>  }
>  
> -void
> +int
>  fs_table_insert_project_path(
>  	char		*dir,
>  	prid_t		prid)
> @@ -561,9 +561,8 @@ fs_table_insert_project_path(
>  	else
>  		error = ENOENT;
>  
> -	if (error) {
> +	if (error)
>  		fprintf(stderr, _("%s: cannot setup path for project dir %s: %s\n"),
>  				progname, dir, strerror(error));

Why not move this to the (sole) caller?  Libraries (even pseudolibraries
like libfrog) usually aren't supposed to go around fprintfing things.

--D

> -		exit(1);
> -	}
> +	return error;
>  }
> diff --git a/libfrog/paths.h b/libfrog/paths.h
> index c08e373..f20a2c3 100644
> --- a/libfrog/paths.h
> +++ b/libfrog/paths.h
> @@ -40,7 +40,7 @@ extern char *mtab_file;
>  extern void fs_table_initialise(int, char *[], int, char *[]);
>  extern void fs_table_destroy(void);
>  
> -extern void fs_table_insert_project_path(char *__dir, uint __projid);
> +extern int fs_table_insert_project_path(char *__dir, uint __projid);
>  
>  
>  extern fs_path_t *fs_table_lookup(const char *__dir, uint __flags);
> diff --git a/quota/project.c b/quota/project.c
> index 03ae10d..bed0dc5 100644
> --- a/quota/project.c
> +++ b/quota/project.c
> @@ -281,7 +281,9 @@ project_f(
>  			break;
>  		case 'p':
>  			ispath = 1;
> -			fs_table_insert_project_path(optarg, -1);
> +			/* fs_table_insert_project_path prints the failure */
> +			if (fs_table_insert_project_path(optarg, -1))
> +				return 0;
>  			break;
>  		case 's':
>  			type = SETUP_PROJECT;
> -- 
> 1.8.3.1
>
Eric Sandeen Dec. 11, 2021, 5:52 a.m. UTC | #2 
On 12/10/21 6:21 PM, Darrick J. Wong wrote:
> On Fri, Dec 10, 2021 at 02:21:36PM -0600, Eric Sandeen wrote:
>> From: Eric Sandeen <sandeen@redhat.com>
>>
>> If "project -p" fails in fs_table_insert_project_path, it
>> calls exit() today which is quite unfriendly. Return an error
>> and return to the command prompt as expected.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
>> ---
>>   libfrog/paths.c | 7 +++----
>>   libfrog/paths.h | 2 +-
>>   quota/project.c | 4 +++-
>>   3 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/libfrog/paths.c b/libfrog/paths.c
>> index d679376..6c0fee2 100644
>> --- a/libfrog/paths.c
>> +++ b/libfrog/paths.c
>> @@ -546,7 +546,7 @@ out_error:
>>   		progname, strerror(error));
>>   }
>>   
>> -void
>> +int
>>   fs_table_insert_project_path(
>>   	char		*dir,
>>   	prid_t		prid)
>> @@ -561,9 +561,8 @@ fs_table_insert_project_path(
>>   	else
>>   		error = ENOENT;
>>   
>> -	if (error) {
>> +	if (error)
>>   		fprintf(stderr, _("%s: cannot setup path for project dir %s: %s\n"),
>>   				progname, dir, strerror(error));
> 
> Why not move this to the (sole) caller?  Libraries (even pseudolibraries
> like libfrog) usually aren't supposed to go around fprintfing things.

I mean, that's a legit goal, but

$ grep -rw "printf\|fprintf"  libfrog/ | wc -l
55

but ok, I can reduce it to 54 ;)

-Eric
Darrick J. Wong Dec. 11, 2021, 4:56 p.m. UTC | #3 
On Fri, Dec 10, 2021 at 11:52:54PM -0600, Eric Sandeen wrote:
> On 12/10/21 6:21 PM, Darrick J. Wong wrote:
> > On Fri, Dec 10, 2021 at 02:21:36PM -0600, Eric Sandeen wrote:
> > > From: Eric Sandeen <sandeen@redhat.com>
> > > 
> > > If "project -p" fails in fs_table_insert_project_path, it
> > > calls exit() today which is quite unfriendly. Return an error
> > > and return to the command prompt as expected.
> > > 
> > > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > > Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> > > ---
> > >   libfrog/paths.c | 7 +++----
> > >   libfrog/paths.h | 2 +-
> > >   quota/project.c | 4 +++-
> > >   3 files changed, 7 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/libfrog/paths.c b/libfrog/paths.c
> > > index d679376..6c0fee2 100644
> > > --- a/libfrog/paths.c
> > > +++ b/libfrog/paths.c
> > > @@ -546,7 +546,7 @@ out_error:
> > >   		progname, strerror(error));
> > >   }
> > > -void
> > > +int
> > >   fs_table_insert_project_path(
> > >   	char		*dir,
> > >   	prid_t		prid)
> > > @@ -561,9 +561,8 @@ fs_table_insert_project_path(
> > >   	else
> > >   		error = ENOENT;
> > > -	if (error) {
> > > +	if (error)
> > >   		fprintf(stderr, _("%s: cannot setup path for project dir %s: %s\n"),
> > >   				progname, dir, strerror(error));
> > 
> > Why not move this to the (sole) caller?  Libraries (even pseudolibraries
> > like libfrog) usually aren't supposed to go around fprintfing things.
> 
> I mean, that's a legit goal, but
> 
> $ grep -rw "printf\|fprintf"  libfrog/ | wc -l
> 55
> 
> but ok, I can reduce it to 54 ;)
> 

With that fixed,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> -Eric
diff mbox series

Patch

diff --git a/libfrog/paths.c b/libfrog/paths.c
index d679376..6c0fee2 100644
--- a/libfrog/paths.c
+++ b/libfrog/paths.c
@@ -546,7 +546,7 @@  out_error:
 		progname, strerror(error));
 }
 
-void
+int
 fs_table_insert_project_path(
 	char		*dir,
 	prid_t		prid)
@@ -561,9 +561,8 @@  fs_table_insert_project_path(
 	else
 		error = ENOENT;
 
-	if (error) {
+	if (error)
 		fprintf(stderr, _("%s: cannot setup path for project dir %s: %s\n"),
 				progname, dir, strerror(error));
-		exit(1);
-	}
+	return error;
 }
diff --git a/libfrog/paths.h b/libfrog/paths.h
index c08e373..f20a2c3 100644
--- a/libfrog/paths.h
+++ b/libfrog/paths.h
@@ -40,7 +40,7 @@  extern char *mtab_file;
 extern void fs_table_initialise(int, char *[], int, char *[]);
 extern void fs_table_destroy(void);
 
-extern void fs_table_insert_project_path(char *__dir, uint __projid);
+extern int fs_table_insert_project_path(char *__dir, uint __projid);
 
 
 extern fs_path_t *fs_table_lookup(const char *__dir, uint __flags);
diff --git a/quota/project.c b/quota/project.c
index 03ae10d..bed0dc5 100644
--- a/quota/project.c
+++ b/quota/project.c
@@ -281,7 +281,9 @@  project_f(
 			break;
 		case 'p':
 			ispath = 1;
-			fs_table_insert_project_path(optarg, -1);
+			/* fs_table_insert_project_path prints the failure */
+			if (fs_table_insert_project_path(optarg, -1))
+				return 0;
 			break;
 		case 's':
 			type = SETUP_PROJECT;