On 14 January 2011 ?. 17:18:41 Alexander Hall wrote:
> On 01/14/11 03:12, Vadim Zhukov wrote:
> > +If multiplier was not specified then this value is interpreted as number of
> > +sectors (see
> > +.Fl S ) ,
> > +not number of bytes.
> 
> I'm not entirely happy with that... Maybe jmc@ can help?
> 
> I'd suggest something even simpler:
> 
> "If no multiplier is present,
> .Ar size
> represents the number of sectors (see
> .Fl S ) .

I like it. :) It's in the new version of patch.

> > +The maximum size of an FFS file system is 2,147,483,647 (2^31 \- 1) of
> >  512\-byte blocks, slightly less than 1 TB.
> >  FFS2 file systems can be as large as 64 PB.
> >  Note however that for
> > Index: newfs.c
> > ===================================================================
> > RCS file: /cvs/src/sbin/newfs/newfs.c,v
> > retrieving revision 1.88
> > diff -u -p -r1.88 newfs.c
> > --- newfs.c 13 Dec 2010 00:02:58 -0000      1.88
> > +++ newfs.c 14 Jan 2011 02:09:52 -0000
> > @@ -114,7 +114,7 @@ int     mfs;                    /* run as the memory 
> > based fi
> >  int        Nflag;                  /* run without writing file system */
> >  int        Oflag = 1;              /* 0 = 4.3BSD ffs, 1 = 4.4BSD ffs, 2 = 
> > ffs2 */
> >  daddr64_t  fssize;                 /* file system size */
> > -int        sectorsize;             /* bytes/sector */
> > +long long  sectorsize;             /* bytes/sector */
> >  int        fsize = 0;              /* fragment size */
> >  int        bsize = 0;              /* block size */
> >  int        maxfrgspercg = INT_MAX; /* maximum fragments per cylinder group 
> > */
> > @@ -169,6 +169,8 @@ main(int argc, char *argv[])
> >     char **saveargv = argv;
> >     int ffsflag = 1;
> >     const char *errstr;
> > +   long long fssize_input;
> > +   int fssize_usebytes = 0;
> >  
> >     if (strstr(__progname, "mfs"))
> >             mfs = Nflag = quiet = 1;
> > @@ -192,9 +194,12 @@ main(int argc, char *argv[])
> >                     oflagset = 1;
> >                     break;
> >             case 'S':
> > -                   sectorsize = strtonum(optarg, 1, INT_MAX, &errstr);
> > -                   if (errstr)
> > -                           fatal("sector size is %s: %s", errstr, optarg);
> > +                   if (scan_scaled(optarg, &sectorsize) == -1)
> > +                           fatal("sector size: %s: %s",
> > +                               strerror(errno), optarg);
> > +                   if (sectorsize <= 0)
> > +                           fatal("sector size is not positive: %s",
> > +                               optarg);
> >                     break;
> >             case 'T':
> >                     disktype = optarg;
> > @@ -265,10 +270,18 @@ main(int argc, char *argv[])
> >                     quiet = 1;
> >                     break;
> >             case 's':
> > -                   fssize = strtonum(optarg, 1, LLONG_MAX, &errstr);
> > -                   if (errstr)
> > +                   if (scan_scaled(optarg, &fssize_input) == -1)
> >                             fatal("file system size is %s: %s",
> > -                               errstr, optarg);
> > +                               strerror(errno), optarg);
> 
> This looks strange;
> -s xyz => "file system size is invalid argument: zyx" ?
> 
> > +                   if (fssize_input <= 0)
> > +                           fatal("file system size is not positive: %s",
> > +                               optarg);
> 
> Do we really need different ways of just telling us that the size
> argument is invalid?

Hm-m. Looks like you're right. Fixed.

>                       if (scan_scaled(optarg, &fssize_input) == -1 ||
>                           fssize_input <= 0)
>                               fatal("invalid file system size");
> 
> (Up for discussion)
> 
> > +                   fssize_usebytes = 0;    /* in case of multiple -s */
> > +                   for (s1 = optarg; *s1 != '\0'; s1++)
> > +                           if (isalpha(*s1)) {
> > +                                   fssize_usebytes = 1;
> > +                                   break;
> > +                           }
> >                     break;
> >             case 't':
> >                     fstype = optarg;
> > @@ -412,17 +425,25 @@ main(int argc, char *argv[])
> >                           argv[0], *cp);
> >     }
> >  havelabel:
> > -   if (fssize == 0)
> > -           fssize = DL_GETPSIZE(pp);
> > -   if (fssize > DL_GETPSIZE(pp) && !mfs)
> > -          fatal("%s: maximum file system size on the `%c' partition is 
> > %lld",
> > -                   argv[0], *cp, DL_GETPSIZE(pp));
> > -
> >     if (sectorsize == 0) {
> >             sectorsize = lp->d_secsize;
> >             if (sectorsize <= 0)
> >                     fatal("%s: no default sector size", argv[0]);
> >     }
> > +
> > +   if (fssize_usebytes) {
> > +           fssize = (daddr64_t)fssize_input / (daddr64_t)sectorsize;
> > +           if ((daddr64_t)fssize_input % (daddr64_t)sectorsize != 0)
> > +                   fssize++;
> > +   } else if (fssize_input == 0)
> > +           fssize = DL_GETPSIZE(pp);
> > +   else
> > +           fssize = (daddr64_t)fssize_input;
> > +
> > +   if (fssize > DL_GETPSIZE(pp) && !mfs)
> > +          fatal("%s: maximum file system size on the `%c' partition is 
> > %lld",
> 
> Maybe this should be changed into "%lld sectors" and/or the size in bytes?

I added " sectors" word. Also fixed analogious string in mkfs.c, see
below. Probably mkfs.c will need more love (for example, use of fatal()
instead of errx()), but other changes should go as separate
step/commit, yes?

-- 
  Best wishes,
    Vadim Zhukov

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?


Index: mkfs.c
===================================================================
RCS file: /cvs/src/sbin/newfs/mkfs.c,v
retrieving revision 1.74
diff -u -p -r1.74 mkfs.c
--- mkfs.c      21 Mar 2010 09:13:30 -0000      1.74
+++ mkfs.c      14 Jan 2011 18:58:59 -0000
@@ -404,8 +404,8 @@ mkfs(struct partition *pp, char *fsys, i
                lastminfpg = roundup(sblock.fs_iblkno +
                    sblock.fs_ipg / INOPF(&sblock), sblock.fs_frag);
                if (sblock.fs_size < lastminfpg)
-                       errx(28, "file system size %jd < minimum size of %d",
-                           (intmax_t)sblock.fs_size, lastminfpg);
+                       errx(28, "file system size %jd < minimum size of %d "
+                           "sectors", (intmax_t)sblock.fs_size, lastminfpg);
 
                if (sblock.fs_size % sblock.fs_fpg >= lastminfpg ||
                    sblock.fs_size % sblock.fs_fpg == 0)
Index: newfs.8
===================================================================
RCS file: /cvs/src/sbin/newfs/newfs.8,v
retrieving revision 1.68
diff -u -p -r1.68 newfs.8
--- newfs.8     21 Mar 2010 07:51:23 -0000      1.68
+++ newfs.8     14 Jan 2011 18:58:59 -0000
@@ -218,6 +218,8 @@ With this option,
 will not print extraneous information like superblock backups.
 .It Fl S Ar sector-size
 The size of a sector in bytes (almost always 512).
+The argument may contain a multiplier, as documented in
+.Xr scan_scaled 3 .
 A sector is the smallest addressable unit on the physical device.
 Changing this is useful only when using
 .Nm
@@ -230,11 +232,14 @@ from its default will make it impossible
 to find the alternate superblocks if the standard superblock is
 lost.
 .It Fl s Ar size
-The size of the file system in sectors.
-This value is multiplied by the number of 512\-byte blocks in a sector
-to yield the size of the file system in 512\-byte blocks, which is the value
-used by the kernel.
-The maximum size of an FFS file system is 2,147,483,647 (2^31 \- 1) of these
+The size of the file system.
+The argument may contain a multiplier, as documented in
+.Xr scan_scaled 3 .
+If no multiplier is present,
+.Ar size
+represents the number of sectors (see
+.Fl S ) .
+The maximum size of an FFS file system is 2,147,483,647 (2^31 \- 1) of
 512\-byte blocks, slightly less than 1 TB.
 FFS2 file systems can be as large as 64 PB.
 Note however that for
Index: newfs.c
===================================================================
RCS file: /cvs/src/sbin/newfs/newfs.c,v
retrieving revision 1.88
diff -u -p -r1.88 newfs.c
--- newfs.c     13 Dec 2010 00:02:58 -0000      1.88
+++ newfs.c     14 Jan 2011 18:58:59 -0000
@@ -114,7 +114,7 @@ int mfs;                    /* run as the memory based fi
 int    Nflag;                  /* run without writing file system */
 int    Oflag = 1;              /* 0 = 4.3BSD ffs, 1 = 4.4BSD ffs, 2 = ffs2 */
 daddr64_t      fssize;                 /* file system size */
-int    sectorsize;             /* bytes/sector */
+long long      sectorsize;             /* bytes/sector */
 int    fsize = 0;              /* fragment size */
 int    bsize = 0;              /* block size */
 int    maxfrgspercg = INT_MAX; /* maximum fragments per cylinder group */
@@ -169,6 +169,8 @@ main(int argc, char *argv[])
        char **saveargv = argv;
        int ffsflag = 1;
        const char *errstr;
+       long long fssize_input;
+       int fssize_usebytes = 0;
 
        if (strstr(__progname, "mfs"))
                mfs = Nflag = quiet = 1;
@@ -192,9 +194,9 @@ main(int argc, char *argv[])
                        oflagset = 1;
                        break;
                case 'S':
-                       sectorsize = strtonum(optarg, 1, INT_MAX, &errstr);
-                       if (errstr)
-                               fatal("sector size is %s: %s", errstr, optarg);
+                       if (scan_scaled(optarg, &sectorsize) == -1 ||
+                           sectorsize <= 0)
+                               fatal("sector size invalid: %s", optarg);
                        break;
                case 'T':
                        disktype = optarg;
@@ -265,10 +267,15 @@ main(int argc, char *argv[])
                        quiet = 1;
                        break;
                case 's':
-                       fssize = strtonum(optarg, 1, LLONG_MAX, &errstr);
-                       if (errstr)
-                               fatal("file system size is %s: %s",
-                                   errstr, optarg);
+                       if (scan_scaled(optarg, &fssize_input) == -1 ||
+                           fssize_input <= 0)
+                               fatal("file system size invalid: %s", optarg);
+                       fssize_usebytes = 0;    /* in case of multiple -s */
+                       for (s1 = optarg; *s1 != '\0'; s1++)
+                               if (isalpha(*s1)) {
+                                       fssize_usebytes = 1;
+                                       break;
+                               }
                        break;
                case 't':
                        fstype = optarg;
@@ -412,17 +419,25 @@ main(int argc, char *argv[])
                              argv[0], *cp);
        }
 havelabel:
-       if (fssize == 0)
-               fssize = DL_GETPSIZE(pp);
-       if (fssize > DL_GETPSIZE(pp) && !mfs)
-              fatal("%s: maximum file system size on the `%c' partition is 
%lld",
-                       argv[0], *cp, DL_GETPSIZE(pp));
-
        if (sectorsize == 0) {
                sectorsize = lp->d_secsize;
                if (sectorsize <= 0)
                        fatal("%s: no default sector size", argv[0]);
        }
+
+       if (fssize_usebytes) {
+               fssize = (daddr64_t)fssize_input / (daddr64_t)sectorsize;
+               if ((daddr64_t)fssize_input % (daddr64_t)sectorsize != 0)
+                       fssize++;
+       } else if (fssize_input == 0)
+               fssize = DL_GETPSIZE(pp);
+       else
+               fssize = (daddr64_t)fssize_input;
+
+       if (fssize > DL_GETPSIZE(pp) && !mfs)
+              fatal("%s: maximum file system size on the `%c' partition is "
+                  "%lld sectors", argv[0], *cp, DL_GETPSIZE(pp));
+
        fssize *= sectorsize / DEV_BSIZE;
        if (oflagset == 0 && fssize >= INT_MAX)
                Oflag = 2;      /* FFS2 */

Reply via email to