[PATCH] [LIBXCMD]: Move error messages to appropriate places.

[PATCH] [LIBXCMD]: Move error messages to appropriate places.

[Arkadiusz Miśkiewicz]

Replace raw perror() with proper message in
fs_table_initialise_mounts().

Replace raw perror() with proper message and move it from
fs_mount_point_from_path() to fs_table_initialise_projects() where this
message has meaning.

Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
---
 libxcmd/paths.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/libxcmd/paths.c b/libxcmd/paths.c
index facea06..6c36b79 100644
--- a/libxcmd/paths.c
+++ b/libxcmd/paths.c
@@ -248,7 +248,8 @@ fs_table_initialise_mounts(
 	dir = fsname = fslog = fsrt = NULL;
 
 	if ((count = getmntinfo(&stats, 0)) < 0) {
-		perror("getmntinfo");
+		fprintf(stderr, _("%s: getmntinfo() failed: %s\n"),
+				progname, strerror(errno));
 		return 0;
 	}
 
@@ -298,7 +299,6 @@ fs_mount_point_from_path(
 	struct stat64	s;
 
 	if (stat64(dir, &s) < 0) {
-		perror(dir);
 		return NULL;
 	}
 
@@ -327,8 +327,11 @@ fs_table_initialise_projects(
 	while ((path = getprpathent()) != NULL) {
 		if (project && prid != path->pp_prid)
 			continue;
-		if ((fs = fs_mount_point_from_path(path->pp_pathname)) == NULL)
+		if ((fs = fs_mount_point_from_path(path->pp_pathname)) == NULL) {
+			fprintf(stderr, _("%s: cannot find mount point for path `%s': %s\n"),
+					progname, path->pp_pathname, strerror(errno));
 			continue;
+		}
 		found = 1;
 		dir = strdup(path->pp_pathname);
 		fsname = strdup(fs->fs_name);
-- 
1.6.1.1

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

Re: [PATCH] [LIBXCMD]: Move error messages to appropriate places.

[Nathan Scott]

On Wed, 2009-01-28 at 19:24 +0100, Arkadiusz Miśkiewicz wrote:
> Replace raw perror() with proper message in
> fs_table_initialise_mounts().
> 
> Replace raw perror() with proper message and move it from
> fs_mount_point_from_path() to fs_table_initialise_projects() where this
> message has meaning.

Looks fine to me.

--
Nathan

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

Re: [PATCH] [LIBXCMD]: Move error messages to appropriate places.

[Donald Douwsma]

Arkadiusz Miśkiewicz wrote:
> Replace raw perror() with proper message in
> fs_table_initialise_mounts().
> 
> Replace raw perror() with proper message and move it from
> fs_mount_point_from_path() to fs_table_initialise_projects() where this
> message has meaning.

I like your change, it makes things clearer.

But does this particular error actually help people? I've seen it a few times
when testing quota code in the past. It's only used when reading in projects,
and causes errors to be displayed even when issuing non project commands.

$ sudo bash -c "echo 10:/var/nonexistantdir >> /etc/projects
$ xfs_quota -c print
/var/nonexistantdir: No such file or directory
Filesystem          Pathname
/home               /dev/sda8

I'd be tempted to remove this error, and add error checking code to the project specific
commands that walk tree's.

Unless there's some corner case I'm missing, What do you think?

Don

> 
> Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
> ---
>  libxcmd/paths.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/libxcmd/paths.c b/libxcmd/paths.c
> index facea06..6c36b79 100644
> --- a/libxcmd/paths.c
> +++ b/libxcmd/paths.c
> @@ -248,7 +248,8 @@ fs_table_initialise_mounts(
>  	dir = fsname = fslog = fsrt = NULL;
>  
>  	if ((count = getmntinfo(&stats, 0)) < 0) {
> -		perror("getmntinfo");
> +		fprintf(stderr, _("%s: getmntinfo() failed: %s\n"),
> +				progname, strerror(errno));
>  		return 0;
>  	}
>  
> @@ -298,7 +299,6 @@ fs_mount_point_from_path(
>  	struct stat64	s;
>  
>  	if (stat64(dir, &s) < 0) {
> -		perror(dir);
>  		return NULL;
>  	}
>  
> @@ -327,8 +327,11 @@ fs_table_initialise_projects(
>  	while ((path = getprpathent()) != NULL) {
>  		if (project && prid != path->pp_prid)
>  			continue;
> -		if ((fs = fs_mount_point_from_path(path->pp_pathname)) == NULL)
> +		if ((fs = fs_mount_point_from_path(path->pp_pathname)) == NULL) {
> +			fprintf(stderr, _("%s: cannot find mount point for path `%s': %s\n"),
> +					progname, path->pp_pathname, strerror(errno));
>  			continue;
> +		}
>  		found = 1;
>  		dir = strdup(path->pp_pathname);
>  		fsname = strdup(fs->fs_name);


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

Re: [PATCH] [LIBXCMD]: Move error messages to appropriate places.

[Arkadiusz Miskiewicz]

On Thursday 29 of January 2009, Donald Douwsma wrote:
> Arkadiusz Miśkiewicz wrote:
> > Replace raw perror() with proper message in
> > fs_table_initialise_mounts().
> >
> > Replace raw perror() with proper message and move it from
> > fs_mount_point_from_path() to fs_table_initialise_projects() where this
> > message has meaning.
>
> I like your change, it makes things clearer.
>
> But does this particular error actually help people? I've seen it a few
> times when testing quota code in the past. It's only used when reading in
> projects, and causes errors to be displayed even when issuing non project
> commands.
>
> $ sudo bash -c "echo 10:/var/nonexistantdir >> /etc/projects
> $ xfs_quota -c print
> /var/nonexistantdir: No such file or directory
> Filesystem          Pathname
> /home               /dev/sda8

Now it will be:
xfs_quota: cannot find mount point for path `/var/nonexistantdir': No such 
file or directory

> I'd be tempted to remove this error, and add error checking code to the
> project specific commands that walk tree's.

The path won't be added to walk iterator and there will be no error at walk 
time. 

The fix would be probably to not add project to the iterating engine when 
asking for non-project things etc. 

> Don

-- 
Arkadiusz Miśkiewicz        PLD/Linux Team
arekm / maven.pl            http://ftp.pld-linux.org/

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

Re: [PATCH] [LIBXCMD]: Move error messages to appropriate places.

[Donald Douwsma]

Arkadiusz Miskiewicz wrote:
> On Thursday 29 of January 2009, Donald Douwsma wrote:
>> Arkadiusz Miśkiewicz wrote:
>>> Replace raw perror() with proper message in
>>> fs_table_initialise_mounts().
>>>
>>> Replace raw perror() with proper message and move it from
>>> fs_mount_point_from_path() to fs_table_initialise_projects() where this
>>> message has meaning.
>> I like your change, it makes things clearer.
>>
>> But does this particular error actually help people? I've seen it a few
>> times when testing quota code in the past. It's only used when reading in
>> projects, and causes errors to be displayed even when issuing non project
>> commands.
>>
>> $ sudo bash -c "echo 10:/var/nonexistantdir >> /etc/projects
>> $ xfs_quota -c print
>> /var/nonexistantdir: No such file or directory
>> Filesystem          Pathname
>> /home               /dev/sda8
> 
> Now it will be:
> xfs_quota: cannot find mount point for path `/var/nonexistantdir': No such 
> file or directory
> 
>> I'd be tempted to remove this error, and add error checking code to the
>> project specific commands that walk tree's.
> 
> The path won't be added to walk iterator and there will be no error at walk 
> time. 

Good point, if we dont give the error at that point there is no way for the user to
find out what happened, not good.

> 
> The fix would be probably to not add project to the iterating engine when 
> asking for non-project things etc. 

That would be nice, in the meantime I think your change is a good idea.

Don

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

Re: [PATCH] [LIBXCMD]: Move error messages to appropriate places.

[Christoph Hellwig]

Thanks, I've put this one in.

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