Re: [PATCH 1/2] fdisk: warn if opening a device in write mode failed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2014-02-27 at 04:26 +0100, Maciej Małecki wrote:
> Otherwise, `fdisk` fails silently with "Bad file descriptor" when
> writing the partition table.
> 
> Signed-off-by: Maciej Małecki <me@xxxxxxxxxxxx>
> ---
>  libfdisk/src/context.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/libfdisk/src/context.c b/libfdisk/src/context.c
> index c405403..40f9080 100644
> --- a/libfdisk/src/context.c
> +++ b/libfdisk/src/context.c
> @@ -248,17 +248,26 @@ static int warn_wipe(struct fdisk_context *cxt)
>  int fdisk_context_assign_device(struct fdisk_context *cxt,
>  				const char *fname, int readonly)
>  {
> -	int fd;
> +	int fd, mode;
>  
>  	DBG(CONTEXT, dbgprint("assigning device %s", fname));
>  	assert(cxt);
>  
>  	reset_context(cxt);
>  
> -	if (readonly == 1 || (fd = open(fname, O_RDWR|O_CLOEXEC)) < 0) {
> -		if ((fd = open(fname, O_RDONLY|O_CLOEXEC)) < 0)
> +retry:
> +	mode = (readonly ? O_RDONLY : O_RDWR) | O_CLOEXEC;
> +	fd = open(fname, mode);
> +	if (fd < 0) {
> +		if (readonly)
>  			return -errno;
> -		readonly = 1;
> +		else {
> +			fdisk_warn(cxt,
> +			    _("%s: opening device in read write mode failed"),
> +			    fname);
> +			readonly = 1;
> +			goto retry;
> +		}
>  	}

This is 1) obscenely ugly and 2) not the place for such message. There's
no reason to pollute output like this. Why do I care how the device is
opened when I'm not going to write anything to it?

The correct way is to do what you do in patch 2, inform the user that
the device is only for opened for reading and just skip the write
operation.

So, NAK for this patch.

Thanks,
Davidlohr


--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux