Re: bin/54201: disklabel(8) annoyingly overrides user intent when run as a "native" tool

[mlelstv%serpens.de@localhost (Michael van Elst)]

The following reply was made to PR bin/54201; it has been noted by GNATS.

From: mlelstv%serpens.de@localhost (Michael van Elst)
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: bin/54201: disklabel(8) annoyingly overrides user intent when run as a "native" tool
Date: Mon, 13 May 2019 05:14:05 -0000 (UTC)

 thorpej%me.com@localhost writes:
 
 >The handling of the -m flag seems unnecessarily complex in the disklabel(8) code, and also seems that how it works to lead to user confusion.
 
 Maybe this:
 
 Index: main.c
 ===================================================================
 RCS file: /cvsroot/src/sbin/disklabel/main.c,v
 retrieving revision 1.50
 diff -p -u -r1.50 main.c
 --- main.c	27 Jun 2018 01:14:48 -0000	1.50
 +++ main.c	13 May 2019 05:12:38 -0000
 @@ -154,7 +154,6 @@ static	int	tflag;		/* Format output as d
  static	int	Dflag;		/* Delete old labels (use with write) */
  static	int	Iflag;		/* Read/write direct, but default if absent */
  static	int	lflag;		/* List all known file system types and exit */
 -static	int	mflag;		/* Expect disk to contain an MBR */
  static int verbose;
  static int read_all;		/* set if op = READ && Aflag */
  
 @@ -481,6 +480,7 @@ main(int argc, char *argv[])
  #endif
  		DELETE
  	} op = UNSPEC, old_op;
 +	unsigned long val;
  
  #ifndef HAVE_NBTOOL_CONFIG_H
  #if !defined(NATIVELABEL_ONLY)
 @@ -502,22 +502,13 @@ main(int argc, char *argv[])
  	}
  #endif
  
 -	mflag = labelusesmbr;
 -	if (mflag < 0) {
 -#if HAVE_NBTOOL_CONFIG_H
 -		warn("getlabelusesmbr() failed");
 -#else
 -		warn("getlabelusesmbr() failed");
 -		mflag = LABELUSESMBR;
 -#endif
 -	}
  #if HAVE_NBTOOL_CONFIG_H
  	/* We must avoid doing any ioctl requests */
  	Fflag = rflag = 1;
  #endif
  
  	error = 0;
 -	while ((ch = getopt(argc, argv, "AB:CDFIM:NRWef:ilmrtvw")) != -1) {
 +	while ((ch = getopt(argc, argv, "AB:CDFIL:M:NO:P:RWef:ilmnrtvw")) != -1) {
  		old_op = op;
  		switch (ch) {
  		case 'A':	/* Action all labels */
 @@ -559,6 +550,24 @@ main(int argc, char *argv[])
  		case 'N':	/* Disallow writes to label sector */
  			op = SETREADONLY;
  			break;
 +		case 'L':	/* Label sector */
 +			val = strtoul(optarg, NULL, 10);
 +			if ((val == ULONG_MAX && errno == ERANGE) || val > UINT_MAX)
 +				err(EXIT_FAILURE, "invalid label sector: %s", optarg);
 +			labelsector = val;
 +			break;
 +		case 'O':	/* Label offset */
 +			val = strtoul(optarg, NULL, 10);
 +			if ((val == ULONG_MAX && errno == ERANGE) || val > UINT_MAX)
 +				err(EXIT_FAILURE, "invalid label offset: %s", optarg);
 +			labeloffset = val;
 +			break;
 +		case 'P':	/* Max partitions */
 +			val = strtoul(optarg, NULL, 10);
 +			if ((val == ULONG_MAX && errno == ERANGE) || val > UINT_MAX)
 +				err(EXIT_FAILURE, "invalid max partitions: %s", optarg);
 +			maxpartitions = val;
 +			break;
  		case 'W':	/* Allow writes to label sector */
  			op = SETWRITABLE;
  			break;
 @@ -578,7 +587,10 @@ main(int argc, char *argv[])
  			lflag = 1;
  			break;
  		case 'm':	/* Expect disk to have an MBR */
 -			mflag ^= 1;
 +			labelusesmbr = 1;
 +			break;
 +		case 'n':	/* Expect disk to not have an MBR */
 +			labelusesmbr = 0;
  			break;
  		case 'r':	/* Read/write label directly from disk */
  			rflag = 1;
 @@ -989,7 +1001,7 @@ readlabel_mbr(int f, u_int sector)
  static int
  writelabel_mbr(int f, u_int sector)
  {
 -	return update_label(f, sector, mflag ? LABELOFFSET_MBR : ~0U) ? 2 : 0;
 +	return update_label(f, sector, labelusesmbr ? LABELOFFSET_MBR : ~0U) ? 2 : 0;
  }
  
  #endif	/* !NO_MBR_SUPPORT */
 @@ -1369,7 +1381,7 @@ readlabel_direct(int f)
  		}
  	}
  
 -	if (mflag && process_mbr(f, readlabel_mbr) == 0)
 +	if (labelusesmbr && process_mbr(f, readlabel_mbr) == 0)
  		return 0;
  
  	disk_lp = find_label(f, 0);
 @@ -1378,7 +1390,7 @@ readlabel_direct(int f)
  		return 0;
  	}
  
 -	if (!mflag && process_mbr(f, readlabel_mbr) == 0)
 +	if (!labelusesmbr && process_mbr(f, readlabel_mbr) == 0)
  		return 0;
  
  	return 1;
 
 -- 
 -- 
                                 Michael van Elst
 Internet: mlelstv%serpens.de@localhost
                                 "A potential Snark may lurk in every tree."