Subject: [PATCH] mount.cifs: remove redundant error assignment

Subject: [PATCH] mount.cifs: remove redundant error assignment

[Suresh Jayaraman]

... as it is done anyway at add_mtab_exit.

Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
 mount.cifs.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/mount.cifs.c b/mount.cifs.c
index 9f04261..49fdd08 100644
--- a/mount.cifs.c
+++ b/mount.cifs.c
@@ -1513,7 +1513,6 @@ add_mtab(char *devname, char *mountpoint, unsigned long flags, const char *fstyp
 	rc = lock_mtab();
 	if (rc) {
 		fprintf(stderr, "cannot lock mtab");
-		rc = EX_FILEIO;
 		goto add_mtab_exit;
 	}
 
@@ -1521,7 +1520,6 @@ add_mtab(char *devname, char *mountpoint, unsigned long flags, const char *fstyp
 	if (!pmntfile) {
 		fprintf(stderr, "could not update mount table\n");
 		unlock_mtab();
-		rc = EX_FILEIO;
 		goto add_mtab_exit;
 	}

Re: Subject: [PATCH] mount.cifs: remove redundant error assignment

[Jeff Layton]

On Wed, 04 Aug 2010 12:31:40 +0530
Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:

> 
> ... as it is done anyway at add_mtab_exit.
> 
> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> ---
>  mount.cifs.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/mount.cifs.c b/mount.cifs.c
> index 9f04261..49fdd08 100644
> --- a/mount.cifs.c
> +++ b/mount.cifs.c
> @@ -1513,7 +1513,6 @@ add_mtab(char *devname, char *mountpoint, unsigned long flags, const char *fstyp
>  	rc = lock_mtab();
>  	if (rc) {
>  		fprintf(stderr, "cannot lock mtab");
> -		rc = EX_FILEIO;
>  		goto add_mtab_exit;
>  	}
>  
		^^^^^^^^^^^
This seems to be fine.

> @@ -1521,7 +1520,6 @@ add_mtab(char *devname, char *mountpoint, unsigned long flags, const char *fstyp
>  	if (!pmntfile) {
>  		fprintf(stderr, "could not update mount table\n");
>  		unlock_mtab();
> -		rc = EX_FILEIO;
>  		goto add_mtab_exit;
>  	}
		^^^^^^^^^^^
This looks wrong. If we don't set rc here, then it'll still be 0 and
the code at add_mtab_exit won't set it to EX_FILEIO.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: Subject: [PATCH] mount.cifs: remove redundant error assignment

[Suresh Jayaraman]

On 08/04/2010 04:23 PM, Jeff Layton wrote:
> On Wed, 04 Aug 2010 12:31:40 +0530
> Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> 
>> @@ -1521,7 +1520,6 @@ add_mtab(char *devname, char *mountpoint, unsigned long flags, const char *fstyp
>>  	if (!pmntfile) {
>>  		fprintf(stderr, "could not update mount table\n");
>>  		unlock_mtab();
>> -		rc = EX_FILEIO;
>>  		goto add_mtab_exit;
>>  	}
> 		^^^^^^^^^^^
> This looks wrong. If we don't set rc here, then it'll still be 0 and
> the code at add_mtab_exit won't set it to EX_FILEIO.
> 

My bad.. I guess it's due to the effect of working with multiple code versions..

Here's another version which is slightly different.. 


From: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
Subject: [PATCH] mount.cifs: remove redundant error assignment

Avoid setting error code twice by moving error handling out of add_mtab_exit
block. We already set error code and report error in other places.

Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
 mount.cifs.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/mount.cifs.c b/mount.cifs.c
index 9f04261..3623e76 100644
--- a/mount.cifs.c
+++ b/mount.cifs.c
@@ -1554,16 +1554,16 @@ add_mtab(char *devname, char *mountpoint, unsigned long flags, const char *fstyp
 	mountent.mnt_freq = 0;
 	mountent.mnt_passno = 0;
 	rc = addmntent(pmntfile, &mountent);
+	if (rc) {
+		fprintf(stderr, "unable to add mount entry to mtab\n");
+		rc = EX_FILEIO;
+	}
 	endmntent(pmntfile);
 	unlock_mtab();
 	SAFE_FREE(mountent.mnt_opts);
 add_mtab_exit:
 	toggle_dac_capability(1, 0);
 	sigprocmask(SIG_SETMASK, &oldmask, NULL);
-	if (rc) {
-		fprintf(stderr, "unable to add mount entry to mtab\n");
-		rc = EX_FILEIO;
-	}
 
 	return rc;
 }

Re: Subject: [PATCH] mount.cifs: remove redundant error assignment

[Jeff Layton]

On Wed, 04 Aug 2010 16:55:50 +0530
Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:

> On 08/04/2010 04:23 PM, Jeff Layton wrote:
> > On Wed, 04 Aug 2010 12:31:40 +0530
> > Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> > 
> >> @@ -1521,7 +1520,6 @@ add_mtab(char *devname, char *mountpoint, unsigned long flags, const char *fstyp
> >>  	if (!pmntfile) {
> >>  		fprintf(stderr, "could not update mount table\n");
> >>  		unlock_mtab();
> >> -		rc = EX_FILEIO;
> >>  		goto add_mtab_exit;
> >>  	}
> > 		^^^^^^^^^^^
> > This looks wrong. If we don't set rc here, then it'll still be 0 and
> > the code at add_mtab_exit won't set it to EX_FILEIO.
> > 
> 
> My bad.. I guess it's due to the effect of working with multiple code versions..
> 
> Here's another version which is slightly different.. 
> 
> 
> From: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> Subject: [PATCH] mount.cifs: remove redundant error assignment
> 
> Avoid setting error code twice by moving error handling out of add_mtab_exit
> block. We already set error code and report error in other places.
> 
> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> ---
>  mount.cifs.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/mount.cifs.c b/mount.cifs.c
> index 9f04261..3623e76 100644
> --- a/mount.cifs.c
> +++ b/mount.cifs.c
> @@ -1554,16 +1554,16 @@ add_mtab(char *devname, char *mountpoint, unsigned long flags, const char *fstyp
>  	mountent.mnt_freq = 0;
>  	mountent.mnt_passno = 0;
>  	rc = addmntent(pmntfile, &mountent);
> +	if (rc) {
> +		fprintf(stderr, "unable to add mount entry to mtab\n");
> +		rc = EX_FILEIO;
> +	}
>  	endmntent(pmntfile);
>  	unlock_mtab();
>  	SAFE_FREE(mountent.mnt_opts);
>  add_mtab_exit:
>  	toggle_dac_capability(1, 0);
>  	sigprocmask(SIG_SETMASK, &oldmask, NULL);
> -	if (rc) {
> -		fprintf(stderr, "unable to add mount entry to mtab\n");
> -		rc = EX_FILEIO;
> -	}
>  
>  	return rc;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

That looks more reasonable...committed.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>