[PATCH 3/4] fdisk: rework fatal errors

[PATCH 3/4] fdisk: rework fatal errors

[Davidlohr Bueso]

From: Davidlohr Bueso <dave@gnu.org>

When the device cannot be opened, there's no point calling fatal() when we can just use err(3). When any other kind of fatal
error occurs it's Ok, in addition we can also go ahead and close the descriptor before exiting the program as it's currently leaking.

Signed-off-by: Davidlohr Bueso <dave@gnu.org>
---
 fdisk/fdisk.c |   18 ++++++------------
 fdisk/fdisk.h |    9 ++++++---
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/fdisk/fdisk.c b/fdisk/fdisk.c
index f49efd0..a16b672 100644
--- a/fdisk/fdisk.c
+++ b/fdisk/fdisk.c
@@ -264,10 +264,8 @@ static void __attribute__ ((__noreturn__)) usage(FILE *out)
 
 void fatal(enum failure why)
 {
+	close(fd);
 	switch (why) {
-		case unable_to_open:
-			err(EXIT_FAILURE, _("unable to open %s"), disk_device);
-
 		case unable_to_read:
 			err(EXIT_FAILURE, _("unable to read %s"), disk_device);
 
@@ -1087,18 +1085,14 @@ static int get_boot(int try_only) {
 	disklabel = ANY_LABEL;
 	memset(MBRbuffer, 0, 512);
 
-	if (try_only && (fd = open(disk_device, O_RDONLY)) < 0) {
-		fprintf(stderr, _("Cannot open %s\n"), disk_device);
-		fatal(unable_to_open);
-	}
+	if (try_only && (fd = open(disk_device, O_RDONLY)) < 0)
+		err(EXIT_FAILURE, _("unable to open %s"), disk_device);
 	else {
 		if ((fd = open(disk_device, O_RDWR)) < 0) {
 			/* ok, can we read-only the device? */
 			if ((fd = open(disk_device, O_RDONLY)) < 0)
-				fatal(unable_to_open);
-			else
-				printf(_("You will not be able to write "
-					 "the partition table.\n"));
+				err(EXIT_FAILURE, _("unable to open %s"), disk_device);
+			printf(_("You will not be able to write the partition table.\n"));
 		}
 	}
 
@@ -2958,7 +2952,7 @@ main(int argc, char **argv) {
 		for (j = optind; j < argc; j++) {
 			disk_device = argv[j];
 			if ((fd = open(disk_device, O_RDONLY)) < 0)
-				fatal(unable_to_open);
+				err(EXIT_FAILURE, _("unable to open %s"), disk_device);
 			if (blkdev_get_sectors(fd, &size) == -1)
 				fatal(ioctl_error);
 			close(fd);
diff --git a/fdisk/fdisk.h b/fdisk/fdisk.h
index 01e3d72..cff6b60 100644
--- a/fdisk/fdisk.h
+++ b/fdisk/fdisk.h
@@ -44,9 +44,12 @@ enum menutype {
 	EXPERT_MENU,
 };
 
-enum failure {ioctl_error,
-	unable_to_open, unable_to_read, unable_to_seek,
-	unable_to_write};
+enum failure {
+	ioctl_error,
+	unable_to_read, 
+	unable_to_seek,
+	unable_to_write
+};
 
 struct geom {
 	unsigned int heads;
-- 
1.7.4.1

Re: [PATCH 3/4] fdisk: rework fatal errors

[Petr Uzel]

[-- Attachment #1: Type: text/plain, Size: 2761 bytes --]

On Fri, Apr 27, 2012 at 01:23:51PM +0200, Davidlohr Bueso wrote:
> From: Davidlohr Bueso <dave@gnu.org>
> 
> When the device cannot be opened, there's no point calling fatal() when we can just use err(3). When any other kind of fatal
> error occurs it's Ok, in addition we can also go ahead and close the descriptor before exiting the program as it's currently leaking.
> 
> Signed-off-by: Davidlohr Bueso <dave@gnu.org>
> ---
>  fdisk/fdisk.c |   18 ++++++------------
>  fdisk/fdisk.h |    9 ++++++---
>  2 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/fdisk/fdisk.c b/fdisk/fdisk.c
> index f49efd0..a16b672 100644
> --- a/fdisk/fdisk.c
> +++ b/fdisk/fdisk.c
> @@ -264,10 +264,8 @@ static void __attribute__ ((__noreturn__)) usage(FILE *out)
>  
>  void fatal(enum failure why)
>  {
> +	close(fd);
>  	switch (why) {
> -		case unable_to_open:
> -			err(EXIT_FAILURE, _("unable to open %s"), disk_device);
> -
>  		case unable_to_read:
>  			err(EXIT_FAILURE, _("unable to read %s"), disk_device);
>  
> @@ -1087,18 +1085,14 @@ static int get_boot(int try_only) {
>  	disklabel = ANY_LABEL;
>  	memset(MBRbuffer, 0, 512);
>  
> -	if (try_only && (fd = open(disk_device, O_RDONLY)) < 0) {
> -		fprintf(stderr, _("Cannot open %s\n"), disk_device);
> -		fatal(unable_to_open);
> -	}
> +	if (try_only && (fd = open(disk_device, O_RDONLY)) < 0)
> +		err(EXIT_FAILURE, _("unable to open %s"), disk_device);
>  	else {
>  		if ((fd = open(disk_device, O_RDWR)) < 0) {
>  			/* ok, can we read-only the device? */
>  			if ((fd = open(disk_device, O_RDONLY)) < 0)
> -				fatal(unable_to_open);
> -			else
> -				printf(_("You will not be able to write "
> -					 "the partition table.\n"));
> +				err(EXIT_FAILURE, _("unable to open %s"), disk_device);
> +			printf(_("You will not be able to write the partition table.\n"));
>  		}
>  	}
>  
> @@ -2958,7 +2952,7 @@ main(int argc, char **argv) {
>  		for (j = optind; j < argc; j++) {
>  			disk_device = argv[j];
>  			if ((fd = open(disk_device, O_RDONLY)) < 0)
> -				fatal(unable_to_open);
> +				err(EXIT_FAILURE, _("unable to open %s"), disk_device);
>  			if (blkdev_get_sectors(fd, &size) == -1)
>  				fatal(ioctl_error);
>  			close(fd);
> diff --git a/fdisk/fdisk.h b/fdisk/fdisk.h
> index 01e3d72..cff6b60 100644
> --- a/fdisk/fdisk.h
> +++ b/fdisk/fdisk.h
> @@ -44,9 +44,12 @@ enum menutype {
>  	EXPERT_MENU,
>  };
>  
> -enum failure {ioctl_error,
> -	unable_to_open, unable_to_read, unable_to_seek,
> -	unable_to_write};
> +enum failure {
> +	ioctl_error,
> +	unable_to_read, 

Trailing whitespace ^^ :)

Reviewed-by: Petr Uzel <petr.uzel@suse.cz>


Petr

-- 
Petr Uzel
IRC: ptr_uzl @ freenode

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 3/4] fdisk: rework fatal errors

[Karel Zak]

On Fri, Apr 27, 2012 at 01:23:51PM +0200, Davidlohr Bueso wrote:
>  fdisk/fdisk.c |   18 ++++++------------
>  fdisk/fdisk.h |    9 ++++++---
>  2 files changed, 12 insertions(+), 15 deletions(-)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com