Hi, > 1 - keep the parsing (parse_partstr) in the "case 's' " block like > before, instead of deferring it at a later step (before change_part() ). > The sooner you can error out because of argument errors, the better. > > 2 - no need to strdup(3) optarg if you can do 1). you have to check all options first, because the disk is the only argument. Without disk, you cannot determine size, and then you cannot compute percentages. > 3 - missing lower bound check on acc in decimal(): To be honest: I did not really think about that. I took out the check because decimal() also accepts negative sizes, though I don't know why. But you're right, for percentages, you can switch this on. > 4 - documentation does not look right, it's "cyl/mb/gb/tb" (or > "CYL/MB/GB/TB"). "c/m/g/t" are not valid according to the various > strncasecmp() in the code. Because of the 'n' it is - only the length of the modifier of the number is taken, so this works. > 5 - decimal() and parse_smodifier() have a lot to share, so I am against > copy/pasting their code in two different functions. You can merge both, > or at least have a common parse_digit() function. This would avoid typo > like the one for 3). > > 6 - you have a copy/paste typo in parse_partstr(), the second "get > start" comment should read "get size". Fixed that. Ok, in the attachment, you can see a patch that fixes 3, 5 and 6. For 1 and 2, I would rather do it the way I already did. Checking things first, saving the values, and then at some point applying them is imho confusing. Regards, Julian
--- sbin/fdisk/fdisk.c
+++ sbin/fdisk/fdisk.c
@@ -271,10 +271,12 @@
static void get_ptn_alignmemt(void);
#if defined(USE_DISKLIST)
static void get_diskname(const char *, char *, size_t);
#endif
static int change_part(int, int, int, daddr_t, daddr_t, char *);
+static void parse_partstr(char *, int *, daddr_t *, daddr_t *, char **);
+static int64_t parse_smodifier(const char *, char **, int);
static void print_geometry(void);
static int first_active(void);
static void change_active(int);
static void change_bios_geometry(void);
static void dos(int, unsigned char *, unsigned char *, unsigned char *);
@@ -291,10 +293,11 @@
#define DEC_SEC 1 /* asking for a sector number */
#define DEC_RND 2 /* round to end of first track
*/
#define DEC_RND_0 4 /* convert 0 to size of a track
*/
#define DEC_RND_DOWN 8 /* subtract 1 track */
#define DEC_RND_DOWN_2 16 /* subtract 2 tracks */
+#define DEC_RND_BNDY 32 /* round _down_ to boundary, instead of
up. */
static int ptn_id(const char *, int *);
static int type_match(const void *, const void *);
static const char *get_type(int);
static int get_mapping(int, unsigned int *, unsigned int *, unsigned int
*, unsigned long *);
#ifdef BOOTSEL
@@ -329,18 +332,19 @@
{
struct stat sb;
int ch;
size_t len;
char *cp;
+ static char *partstr;
int n;
#ifdef BOOTSEL
daddr_t default_ptn; /* start sector of default ptn */
char *cbootmenu = 0;
#endif
int csysid; /* For the s_flag. */
- unsigned int cstart, csize;
+ daddr_t cstart, csize;
a_flag = u_flag = sh_flag = f_flag = s_flag = b_flag = 0;
i_flag = B_flag = 0;
v_flag = 0;
E_flag = 0;
csysid = cstart = csize = 0;
@@ -398,22 +402,13 @@
case 'v': /* Be verbose */
v_flag++;
break;
case 's': /* Partition details */
s_flag = 1;
- if (sscanf(optarg, "%d/%u/%u%n", &csysid, &cstart,
- &csize, &n) == 3) {
- if (optarg[n] == 0)
- break;
-#ifdef BOOTSEL
- if (optarg[n] == '/') {
- cbootmenu = optarg + n + 1;
- break;
- }
-#endif
- }
- errx(1, "Bad argument to the -s flag.");
+ partstr = strdup(optarg);
+ if (!partstr)
+ err(1, "Could not allocate string.");
break;
case 'b': /* BIOS geometry */
b_flag = 1;
if (sscanf(optarg, "%d/%d/%d%n", &b_cyl, &b_head,
&b_sec, &n) != 3 || optarg[n] != 0)
@@ -528,13 +523,21 @@
/* Do the update stuff! */
if (u_flag) {
if (!f_flag && !b_flag)
change_bios_geometry();
- if (s_flag)
+ if (s_flag) {
+ parse_partstr(partstr, &csysid, &cstart, &csize,
&cbootmenu);
+#ifndef BOOTSEL
+ if (strlen(cbootmenu))
+ errx(1, "Bad argument to the -s flag: Too many
slashes.");
+#endif
+ free(partstr);
+
change_part(E_flag, partition, csysid, cstart, csize,
cbootmenu);
+ }
else {
int part = partition, chg_ext = E_flag, prompt = 1;
do {
if (prompt) {
printf("\n");
@@ -1955,10 +1958,122 @@
}
}
return 0;
}
+/* This function has the same flags as the other round-determining stuff, but
+ * one with a special meaning: DEC_RND_DOWN rounds also to the lower alignment
+ * boundary, instead of the upper one.
+ * We need this to have a total value of 100%. */
+static int64_t
+parse_smodifier(const char *sizestr, char **pos, int flags)
+{
+ char *cp;
+ int64_t acc;
+ int valid;
+ int len;
+
+ if (!isdigit((unsigned char)*sizestr) && *sizestr != '-')
+ return -1;
+
+ acc = strtoll(sizestr, &cp, 10);
+ len = strcspn(cp, " \t\n");
+ valid = 0;
+ if (len != 0) {
+ if (!strncasecmp(cp, "tb", len)) {
+ acc *= 1024;
+ valid = 1;
+ }
+ if (valid || !strncasecmp(cp, "gb", len)) {
+ acc *= 1024;
+ valid = 1;
+ }
+ if (valid || !strncasecmp(cp, "mb", len)) {
+ acc *= SEC_IN_1M;
+ /* round to whole number of cylinders */
+ acc += ptn_alignment / 2;
+ acc /= ptn_alignment;
+ valid = 1;
+ } else if (!strncmp(cp, "%", len)) {
+ if (acc < 0 || acc > 100)
+ return -1;
+ acc = disksectors / 100 * acc;
+ /* round to whole number of cylinders */
+ if (acc <= ptn_alignment / 2)
+ acc += ptn_alignment / 2;
+ else if (flags & DEC_RND_DOWN)
+ acc -= ptn_alignment / 2;
+ else
+ acc += ptn_alignment / 2;
+ acc /= ptn_alignment;
+ valid = 1;
+ }
+ if (valid || !strncasecmp(cp, "cyl", len)) {
+ acc *= ptn_alignment;
+ /* adjustments for cylinder boundary */
+ if (acc == 0 && flags & DEC_RND_0)
+ acc += ptn_0_offset;
+ if (flags & DEC_RND)
+ acc += ptn_0_offset;
+ if (flags & DEC_RND_DOWN)
+ acc -= ptn_0_offset;
+ if (flags & DEC_RND_DOWN_2)
+ acc -= ptn_0_offset;
+ cp += len;
+ }
+ }
+ if (pos != NULL)
+ *pos = cp;
+
+ return acc;
+}
+
+static void
+parse_partstr(char *partstr, int *csysid, daddr_t *cstart,
+ daddr_t *csize, char **cbootmenu)
+{
+ int64_t tval;
+ char *tmpstr;
+
+ /* Get sysid. */
+ tmpstr = strtok(partstr, "/");
+ if (tmpstr == NULL)
+ errx(1, "Bad argument to the -s flag.");
+ *csysid = strtoll(tmpstr, NULL, 10);
+ if (strspn(tmpstr, "1234567890") != strlen(tmpstr))
+ errx(1, "Bad id argument to the -s flag.");
+
+ /* Get start. */
+ tmpstr = strtok(NULL, "/");
+ if (tmpstr == NULL)
+ errx(1, "Bad argument to the -s flag.");
+ tval = parse_smodifier(tmpstr, NULL, DEC_RND);
+ if (tval < 0)
+ errx(1, "Bad start argument to the -s flag.");
+ *cstart = tval;
+
+ /* Get size. */
+ tmpstr = strtok(NULL, "/");
+ if (tmpstr == NULL)
+ errx(1, "Bad argument to the -s flag.");
+ tval = parse_smodifier(tmpstr, NULL, DEC_RND_DOWN);
+ if (tval < 0)
+ errx(1, "Bad size argument to the -s flag.");
+ *csize = tval;
+
+#ifdef BOOTSEL
+ /* Get bootmenu. */
+ tmpstr = strtok(NULL, "/");
+ if (tmpstr != NULL)
+ *cbootmenu = strdup(tmpstr);
+ else
+ *cbootmenu = NULL;
+#endif
+
+ return;
+}
+
static int
change_part(int extended, int part, int sysid, daddr_t start, daddr_t size,
char *bootmenu)
{
struct mbr_partition *partp;
@@ -2761,44 +2876,15 @@
return dflt;
if (cp[0] == '$' && cp[1] == '\n')
return maxval;
- if (isdigit((unsigned char)*cp) || *cp == '-') {
- acc = strtoll(lbuf, &cp, 10);
- len = strcspn(cp, " \t\n");
- valid = 0;
- if (len != 0 && (flags & DEC_SEC)) {
- if (!strncasecmp(cp, "gb", len)) {
- acc *= 1024;
- valid = 1;
- }
- if (valid || !strncasecmp(cp, "mb", len)) {
- acc *= SEC_IN_1M;
- /* round to whole number of cylinders */
- acc += ptn_alignment / 2;
- acc /= ptn_alignment;
- valid = 1;
- }
- if (valid || !strncasecmp(cp, "cyl", len)) {
- acc *= ptn_alignment;
- /* adjustments for cylinder boundary */
- if (acc == 0 && flags & DEC_RND_0)
- acc += ptn_0_offset;
- if (flags & DEC_RND)
- acc += ptn_0_offset;
- if (flags & DEC_RND_DOWN)
- acc -= ptn_0_offset;
- if (flags & DEC_RND_DOWN_2)
- acc -= ptn_0_offset;
- cp += len;
- }
- }
- }
+ if (flags & DEC_SEC)
+ acc = parse_smodifier(cp, &cp, flags);
cp += strspn(cp, " \t");
- if (*cp != '\n') {
+ if (*cp != '\n' || acc == -1) {
lbuf[strlen(lbuf) - 1] = 0;
printf("%s is not a valid %s number.\n", lbuf,
flags & DEC_SEC ? "sector" : "decimal");
continue;
}
Attachment:
signature.asc
Description: PGP signature