[PATCH 0/3] libxfs: tidy up cvtnum junk

[PATCH 0/3] libxfs: tidy up cvtnum junk

[Eric Sandeen]

We have many many patterns like:

int	blocksize, sectorsize;

cvtnum_init(&blocksize, &sectorsize);
cvtnum(blocksize, sectorsize, arg);

where cvtnum_init simply copies filesystem geometry
into those 2 local vars.

Instead, I propose a new wrapper, io_cvtnum(arg) which
just looks to the global file->geom to get these values.

[PATCH 1/3] libxfs: handle 0 blocksize or sectorsize in cvtnum

[Eric Sandeen]

Blocksize and sectorsize are unique in that they must
be provided, unlike every other suffix which can be
calculated from constants.

Nothing protects against unspecified block & sector size,
so catch it if it happens and return a parsing error.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/libxcmd/input.c b/libxcmd/input.c
index 7a69dc1..7b86225 100644
--- a/libxcmd/input.c
+++ b/libxcmd/input.c
@@ -330,8 +330,12 @@ cvtnum(
 	c = tolower(*sp);
 	switch (c) {
 	case 'b':
+		if (!blocksize)
+			return -1LL;
 		return i * blocksize;
 	case 's':
+		if (!sectorsize)
+			return -1LL;
 		return i * sectorsize;
 	case 'k':
 		return KILOBYTES(i);

[PATCH 2/3] xfs_io: get foreign blocksize & sector size in openfile

[Eric Sandeen]

Use statvfs to fill in geometry block size and sector
size in openfile() for "foreign" (non-xfs) files.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/io/open.c b/io/open.c
index f2ea7c3..b0dcabc 100644
--- a/io/open.c
+++ b/io/open.c
@@ -16,6 +16,7 @@
  * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#include <sys/statvfs.h>
 #include "command.h"
 #include "input.h"
 #include "init.h"
@@ -94,13 +95,25 @@ openfile(
                }
        }
 
-       if (!geom || !platform_test_xfs_fd(fd))
+       if (!geom)
                return fd;
 
-       if (xfsctl(path, fd, XFS_IOC_FSGEOMETRY, geom) < 0) {
-               perror("XFS_IOC_FSGEOMETRY");
-               close(fd);
-               return -1;
+       if (platform_test_xfs_fd(fd)) {
+               if (xfsctl(path, fd, XFS_IOC_FSGEOMETRY, geom) < 0) {
+                       perror("XFS_IOC_FSGEOMETRY");
+                       close(fd);
+                       return -1;
+               }
+       } else {        /* Fill in block & sector size for cvtnum */
+               struct statvfs  statbuf;
+
+               if (fstatvfs(fd, &statbuf) < 0) {
+                       perror("fstatvfs");
+                       close(fd);
+                       return -1;
+               }
+               geom->blocksize = statbuf.f_bsize;
+               geom->sectsize = 512;
        }
 
        if (!(flags & IO_READONLY) && (flags & IO_REALTIME)) {

[PATCH 3/3] xfs_io: add io_cvtnum wrapper

[Eric Sandeen]

Now that we set blcok & sector size into each file's geometry,
even for foreign files, add a new wrapper io_cvtnum() which
passes the current file's ->geom into cvtnum() rather than
needing to fill in local vars via cvtnum_init, only to pass
them back into cvtnum().

Switch all cvtnum() to io_cvtnum(), and remove cvtnum_init().

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

 cowextsize.c      |    4 +---
 fadvise.c         |    7 ++-----
 fsmap.c           |    7 ++-----
 init.c            |   19 ++++++++-----------
 init.h            |    2 +-
 madvise.c         |    6 ++----
 mincore.c         |    6 ++----
 mmap.c            |   35 +++++++++++------------------------
 open.c            |   27 +++++++++++++++++++--------
 pread.c           |   11 +++++------
 prealloc.c        |    7 ++-----
 pwrite.c          |   13 ++++++-------
 readdir.c         |    7 ++-----
 reflink.c         |   16 ++++++----------
 resblks.c         |    2 +-
 seek.c            |    4 +---
 sendfile.c        |    6 ++----
 sync_file_range.c |    6 ++----
 truncate.c        |    4 +---
 19 files changed, 76 insertions(+), 113 deletions(-)


diff --git a/io/cowextsize.c b/io/cowextsize.c
index c4cd6de..5eff9ef 100644
--- a/io/cowextsize.c
+++ b/io/cowextsize.c
@@ -143,11 +143,9 @@ cowextsize_f(
 	int		argc,
 	char		**argv)
 {
-	size_t			blocksize, sectsize;
 	int			c;
 
 	recurse_all = recurse_dir = 0;
-	init_cvtnum(&blocksize, &sectsize);
 	while ((c = getopt(argc, argv, "DR")) != EOF) {
 		switch (c) {
 		case 'D':
@@ -164,7 +162,7 @@ cowextsize_f(
 	}
 
 	if (optind < argc) {
-		cowextsize = (long)cvtnum(blocksize, sectsize, argv[optind]);
+		cowextsize = (long)io_cvtnum(argv[optind]);
 		if (cowextsize < 0) {
 			printf(_("non-numeric cowextsize argument -- %s\n"),
 				argv[optind]);
diff --git a/io/fadvise.c b/io/fadvise.c
index 46174f3..9a38ce4 100644
--- a/io/fadvise.c
+++ b/io/fadvise.c
@@ -81,19 +81,16 @@ fadvise_f(
 		}
 	}
 	if (range) {
-		size_t	blocksize, sectsize;
-
 		if (optind != argc - 2)
 			return command_usage(&fadvise_cmd);
-		init_cvtnum(&blocksize, &sectsize);
-		offset = cvtnum(blocksize, sectsize, argv[optind]);
+		offset = io_cvtnum(argv[optind]);
 		if (offset < 0) {
 			printf(_("non-numeric offset argument -- %s\n"),
 				argv[optind]);
 			return 0;
 		}
 		optind++;
-		length = cvtnum(blocksize, sectsize, argv[optind]);
+		length = io_cvtnum(argv[optind]);
 		if (length < 0) {
 			printf(_("non-numeric length argument -- %s\n"),
 				argv[optind]);
diff --git a/io/fsmap.c b/io/fsmap.c
index 9e70c7b..920241d 100644
--- a/io/fsmap.c
+++ b/io/fsmap.c
@@ -396,14 +396,11 @@ fsmap_f(
 	int			i = 0;
 	int			c;
 	unsigned long long	nr = 0;
-	size_t			fsblocksize, fssectsize;
 	struct fs_path		*fs;
 	static bool		tab_init;
 	bool			dumped_flags = false;
 	int			dflag, lflag, rflag;
 
-	init_cvtnum(&fsblocksize, &fssectsize);
-
 	dflag = lflag = rflag = 0;
 	while ((c = getopt(argc, argv, "dlmn:rv")) != EOF) {
 		switch (c) {
@@ -437,7 +434,7 @@ fsmap_f(
 		return command_usage(&fsmap_cmd);
 
 	if (argc > optind) {
-		start = cvtnum(fsblocksize, fssectsize, argv[optind]);
+		start = io_cvtnum(argv[optind]);
 		if (start < 0) {
 			fprintf(stderr,
 				_("Bad rmap start_bblock %s.\n"),
@@ -448,7 +445,7 @@ fsmap_f(
 	}
 
 	if (argc > optind + 1) {
-		end = cvtnum(fsblocksize, fssectsize, argv[optind + 1]);
+		end = io_cvtnum(argv[optind + 1]);
 		if (end < 0) {
 			fprintf(stderr,
 				_("Bad rmap end_bblock %s.\n"),
diff --git a/io/init.c b/io/init.c
index 20d5f80..e54d4c8 100644
--- a/io/init.c
+++ b/io/init.c
@@ -39,18 +39,15 @@ _("Usage: %s [-adfinrRstVx] [-m mode] [-p prog] [[-c|-C] cmd]... file\n"),
 	exit(1);
 }
 
-void
-init_cvtnum(
-	size_t *blocksize,
-	size_t *sectsize)
+long long
+io_cvtnum(
+	char	*s)
 {
-	if (!file || (file->flags & IO_FOREIGN)) {
-		*blocksize = 4096;
-		*sectsize = 512;
-	} else {
-		*blocksize = file->geom.blocksize;
-		*sectsize = file->geom.sectsize;
-	}
+	/* addfile() should always set up this much geometry */
+	ASSERT(file->geom.blocksize);
+	ASSERT(file->geom.sectsize);
+
+	return cvtnum(file->geom.blocksize, file->geom.sectsize, s);
 }
 
 static void
diff --git a/io/init.h b/io/init.h
index bb25242..628e863 100644
--- a/io/init.h
+++ b/io/init.h
@@ -26,4 +26,4 @@ extern int	expert;
 extern size_t	pagesize;
 extern struct timeval stopwatch;
 
-extern void init_cvtnum(size_t *blocksize, size_t *sectsize);
+extern long long io_cvtnum(char *s);
diff --git a/io/madvise.c b/io/madvise.c
index 1d8b53c..456d3b0 100644
--- a/io/madvise.c
+++ b/io/madvise.c
@@ -55,7 +55,6 @@ madvise_f(
 	size_t		length;
 	void		*start;
 	int		advise = MADV_NORMAL, c;
-	size_t		blocksize, sectsize;
 
 	while ((c = getopt(argc, argv, "drsw")) != EOF) {
 		switch (c) {
@@ -80,15 +79,14 @@ madvise_f(
 		offset = mapping->offset;
 		length = mapping->length;
 	} else if (optind == argc - 2) {
-		init_cvtnum(&blocksize, &sectsize);
-		offset = cvtnum(blocksize, sectsize, argv[optind]);
+		offset = io_cvtnum(argv[optind]);
 		if (offset < 0) {
 			printf(_("non-numeric offset argument -- %s\n"),
 				argv[optind]);
 			return 0;
 		}
 		optind++;
-		llength = cvtnum(blocksize, sectsize, argv[optind]);
+		llength = io_cvtnum(argv[optind]);
 		if (llength < 0) {
 			printf(_("non-numeric length argument -- %s\n"),
 				argv[optind]);
diff --git a/io/mincore.c b/io/mincore.c
index 9e0d3a6..179fd4d 100644
--- a/io/mincore.c
+++ b/io/mincore.c
@@ -31,7 +31,6 @@ mincore_f(
 {
 	off64_t		offset, llength;
 	size_t		length;
-	size_t		blocksize, sectsize;
 	void		*start;
 	void		*current, *previous;
 	unsigned char	*vec;
@@ -41,14 +40,13 @@ mincore_f(
 		offset = mapping->offset;
 		length = mapping->length;
 	} else if (argc == 3) {
-		init_cvtnum(&blocksize, &sectsize);
-		offset = cvtnum(blocksize, sectsize, argv[1]);
+		offset = io_cvtnum(argv[1]);
 		if (offset < 0) {
 			printf(_("non-numeric offset argument -- %s\n"),
 				argv[1]);
 			return 0;
 		}
-		llength = cvtnum(blocksize, sectsize, argv[2]);
+		llength = io_cvtnum(argv[2]);
 		if (llength < 0) {
 			printf(_("non-numeric length argument -- %s\n"),
 				argv[2]);
diff --git a/io/mmap.c b/io/mmap.c
index 7a8150e..5bf90e2 100644
--- a/io/mmap.c
+++ b/io/mmap.c
@@ -160,7 +160,6 @@ mmap_f(
 	ssize_t		length = 0, length2 = 0;
 	void		*address = NULL;
 	char		*filename;
-	size_t		blocksize, sectsize;
 	int		c, prot = 0;
 
 	if (argc == 1) {
@@ -182,8 +181,6 @@ mmap_f(
 		return 0;
 	}
 
-	init_cvtnum(&blocksize, &sectsize);
-
 	while ((c = getopt(argc, argv, "rwxs:")) != EOF) {
 		switch (c) {
 		case 'r':
@@ -196,7 +193,7 @@ mmap_f(
 			prot |= PROT_EXEC;
 			break;
 		case 's':
-			length2 = cvtnum(blocksize, sectsize, optarg);
+			length2 = io_cvtnum(optarg);
 			break;
 		default:
 			return command_usage(&mmap_cmd);
@@ -208,13 +205,13 @@ mmap_f(
 	if (optind != argc - 2)
 		return command_usage(&mmap_cmd);
 
-	offset = cvtnum(blocksize, sectsize, argv[optind]);
+	offset = io_cvtnum(argv[optind]);
 	if (offset < 0) {
 		printf(_("non-numeric offset argument -- %s\n"), argv[optind]);
 		return 0;
 	}
 	optind++;
-	length = cvtnum(blocksize, sectsize, argv[optind]);
+	length = io_cvtnum(argv[optind]);
 	if (length < 0) {
 		printf(_("non-numeric length argument -- %s\n"), argv[optind]);
 		return 0;
@@ -292,7 +289,6 @@ msync_f(
 	ssize_t		length;
 	void		*start;
 	int		c, flags = 0;
-	size_t		blocksize, sectsize;
 
 	while ((c = getopt(argc, argv, "ais")) != EOF) {
 		switch (c) {
@@ -314,15 +310,14 @@ msync_f(
 		offset = mapping->offset;
 		length = mapping->length;
 	} else if (optind == argc - 2) {
-		init_cvtnum(&blocksize, &sectsize);
-		offset = cvtnum(blocksize, sectsize, argv[optind]);
+		offset = io_cvtnum(argv[optind]);
 		if (offset < 0) {
 			printf(_("non-numeric offset argument -- %s\n"),
 				argv[optind]);
 			return 0;
 		}
 		optind++;
-		length = cvtnum(blocksize, sectsize, argv[optind]);
+		length = io_cvtnum(argv[optind]);
 		if (length < 0) {
 			printf(_("non-numeric length argument -- %s\n"),
 				argv[optind]);
@@ -378,7 +373,6 @@ mread_f(
 	char		*bp;
 	void		*start;
 	int		dump = 0, rflag = 0, c;
-	size_t		blocksize, sectsize;
 
 	while ((c = getopt(argc, argv, "frv")) != EOF) {
 		switch (c) {
@@ -400,15 +394,14 @@ mread_f(
 		offset = mapping->offset;
 		length = mapping->length;
 	} else if (optind == argc - 2) {
-		init_cvtnum(&blocksize, &sectsize);
-		offset = cvtnum(blocksize, sectsize, argv[optind]);
+		offset = io_cvtnum(argv[optind]);
 		if (offset < 0) {
 			printf(_("non-numeric offset argument -- %s\n"),
 				argv[optind]);
 			return 0;
 		}
 		optind++;
-		length = cvtnum(blocksize, sectsize, argv[optind]);
+		length = io_cvtnum(argv[optind]);
 		if (length < 0) {
 			printf(_("non-numeric length argument -- %s\n"),
 				argv[optind]);
@@ -536,7 +529,6 @@ mwrite_f(
 	int		seed = 'X';
 	int		rflag = 0;
 	int		c;
-	size_t		blocksize, sectsize;
 
 	while ((c = getopt(argc, argv, "rS:")) != EOF) {
 		switch (c) {
@@ -559,15 +551,14 @@ mwrite_f(
 		offset = mapping->offset;
 		length = mapping->length;
 	} else if (optind == argc - 2) {
-		init_cvtnum(&blocksize, &sectsize);
-		offset = cvtnum(blocksize, sectsize, argv[optind]);
+		offset = io_cvtnum(argv[optind]);
 		if (offset < 0) {
 			printf(_("non-numeric offset argument -- %s\n"),
 				argv[optind]);
 			return 0;
 		}
 		optind++;
-		length = cvtnum(blocksize, sectsize, argv[optind]);
+		length = io_cvtnum(argv[optind]);
 		if (length < 0) {
 			printf(_("non-numeric length argument -- %s\n"),
 				argv[optind]);
@@ -620,16 +611,12 @@ mremap_f(
 	void		*new_addr = NULL;
 	int		flags = 0;
 	int		c;
-	size_t		blocksize, sectsize;
-
-	init_cvtnum(&blocksize, &sectsize);
 
 	while ((c = getopt(argc, argv, "f:m")) != EOF) {
 		switch (c) {
 		case 'f':
 			flags = MREMAP_FIXED|MREMAP_MAYMOVE;
-			new_addr = (void *)(unsigned long)cvtnum(blocksize,
-			                          sectsize, optarg);
+			new_addr = (void *)(unsigned long)io_cvtnum(optarg);
 			break;
 		case 'm':
 			flags = MREMAP_MAYMOVE;
@@ -642,7 +629,7 @@ mremap_f(
 	if (optind != argc - 1)
 		return command_usage(&mremap_cmd);
 
-	new_length = cvtnum(blocksize, sectsize, argv[optind]);
+	new_length = io_cvtnum(argv[optind]);
 	if (new_length < 0) {
 		printf(_("non-numeric offset argument -- %s\n"),
 			argv[optind]);
diff --git a/io/open.c b/io/open.c
index f2ea7c3..b0dcabc 100644
--- a/io/open.c
+++ b/io/open.c
@@ -576,11 +589,9 @@ extsize_f(
 	int		argc,
 	char		**argv)
 {
-	size_t			blocksize, sectsize;
 	int			c;
 
 	recurse_all = recurse_dir = 0;
-	init_cvtnum(&blocksize, &sectsize);
 	while ((c = getopt(argc, argv, "DR")) != EOF) {
 		switch (c) {
 		case 'D':
@@ -597,7 +608,7 @@ extsize_f(
 	}
 
 	if (optind < argc) {
-		extsize = (long)cvtnum(blocksize, sectsize, argv[optind]);
+		extsize = (long)io_cvtnum(argv[optind]);
 		if (extsize < 0) {
 			printf(_("non-numeric extsize argument -- %s\n"),
 				argv[optind]);
diff --git a/io/pread.c b/io/pread.c
index 3395503..2d7ccda 100644
--- a/io/pread.c
+++ b/io/pread.c
@@ -381,7 +381,6 @@ pread_f(
 	off64_t		offset;
 	unsigned int	zeed = 0;
 	long long	count, total, tmp;
-	size_t		fsblocksize, fssectsize;
 	struct timeval	t1, t2;
 	char		*sp;
 	int		Cflag, qflag, uflag, vflag;
@@ -389,13 +388,13 @@ pread_f(
 	int		c;
 
 	Cflag = qflag = uflag = vflag = 0;
-	init_cvtnum(&fsblocksize, &fssectsize);
-	bsize = fsblocksize;
+	bsize = file->geom.blocksize;
+	ASSERT(bsize);
 
 	while ((c = getopt(argc, argv, "b:BCFRquvV:Z:")) != EOF) {
 		switch (c) {
 		case 'b':
-			tmp = cvtnum(fsblocksize, fssectsize, optarg);
+			tmp = io_cvtnum(optarg);
 			if (tmp < 0) {
 				printf(_("non-numeric bsize -- %s\n"), optarg);
 				return 0;
@@ -447,7 +446,7 @@ pread_f(
 	if (optind != argc - 2)
 		return command_usage(&pread_cmd);
 
-	offset = cvtnum(fsblocksize, fssectsize, argv[optind]);
+	offset = io_cvtnum(argv[optind]);
 	if (offset < 0 && (direction & (IO_RANDOM|IO_BACKWARD))) {
 		eof = -1;	/* read from EOF */
 	} else if (offset < 0) {
@@ -455,7 +454,7 @@ pread_f(
 		return 0;
 	}
 	optind++;
-	count = cvtnum(fsblocksize, fssectsize, argv[optind]);
+	count = io_cvtnum(argv[optind]);
 	if (count < 0 && (direction & (IO_RANDOM|IO_FORWARD))) {
 		eof = -1;	/* read to EOF */
 	} else if (count < 0) {
diff --git a/io/prealloc.c b/io/prealloc.c
index 1a1c9ca..bade708 100644
--- a/io/prealloc.c
+++ b/io/prealloc.c
@@ -63,17 +63,14 @@ offset_length(
 	char		*length,
 	xfs_flock64_t	*segment)
 {
-	size_t		blocksize, sectsize;
-
-	init_cvtnum(&blocksize, &sectsize);
 	memset(segment, 0, sizeof(*segment));
 	segment->l_whence = SEEK_SET;
-	segment->l_start = cvtnum(blocksize, sectsize, offset);
+	segment->l_start = io_cvtnum(offset);
 	if (segment->l_start < 0) {
 		printf(_("non-numeric offset argument -- %s\n"), offset);
 		return 0;
 	}
-	segment->l_len = cvtnum(blocksize, sectsize, length);
+	segment->l_len = io_cvtnum(length);
 	if (segment->l_len < 0) {
 		printf(_("non-numeric length argument -- %s\n"), length);
 		return 0;
diff --git a/io/pwrite.c b/io/pwrite.c
index 1c5dfca..89b3357 100644
--- a/io/pwrite.c
+++ b/io/pwrite.c
@@ -252,7 +252,6 @@ pwrite_f(
 	off64_t		offset, skip = 0;
 	long long	count, total, tmp;
 	unsigned int	zeed = 0, seed = 0xcdcdcdcd;
-	size_t		fsblocksize, fssectsize;
 	struct timeval	t1, t2;
 	char		*sp, *infile = NULL;
 	int		Cflag, qflag, uflag, dflag, wflag, Wflag;
@@ -260,13 +259,13 @@ pwrite_f(
 	int		c, fd = -1;
 
 	Cflag = qflag = uflag = dflag = wflag = Wflag = 0;
-	init_cvtnum(&fsblocksize, &fssectsize);
-	bsize = fsblocksize;
+	bsize = file->geom.blocksize;
+	ASSERT(bsize);
 
 	while ((c = getopt(argc, argv, "b:BCdf:Fi:qRs:S:uV:wWZ:")) != EOF) {
 		switch (c) {
 		case 'b':
-			tmp = cvtnum(fsblocksize, fssectsize, optarg);
+			tmp = io_cvtnum(optarg);
 			if (tmp < 0) {
 				printf(_("non-numeric bsize -- %s\n"), optarg);
 				return 0;
@@ -293,7 +292,7 @@ pwrite_f(
 			infile = optarg;
 			break;
 		case 's':
-			skip = cvtnum(fsblocksize, fssectsize, optarg);
+			skip = io_cvtnum(optarg);
 			if (skip < 0) {
 				printf(_("non-numeric skip -- %s\n"), optarg);
 				return 0;
@@ -341,13 +340,13 @@ pwrite_f(
 		return command_usage(&pwrite_cmd);
 	if (infile && direction != IO_FORWARD)
 		return command_usage(&pwrite_cmd);
-	offset = cvtnum(fsblocksize, fssectsize, argv[optind]);
+	offset = io_cvtnum(argv[optind]);
 	if (offset < 0) {
 		printf(_("non-numeric offset argument -- %s\n"), argv[optind]);
 		return 0;
 	}
 	optind++;
-	count = cvtnum(fsblocksize, fssectsize, argv[optind]);
+	count = io_cvtnum(argv[optind]);
 	if (count < 0) {
 		printf(_("non-numeric length argument -- %s\n"), argv[optind]);
 		return 0;
diff --git a/io/readdir.c b/io/readdir.c
index ca7a881..af6ea4c 100644
--- a/io/readdir.c
+++ b/io/readdir.c
@@ -140,7 +140,6 @@ readdir_f(
 	int cnt;
 	unsigned long long total;
 	int c;
-	size_t fsblocksize, fssectsize;
 	struct timeval t1, t2;
 	char s1[64], s2[64], ts[64];
 	long long offset = -1;
@@ -149,15 +148,13 @@ readdir_f(
 	DIR *dir;
 	int dfd;
 
-	init_cvtnum(&fsblocksize, &fssectsize);
-
 	while ((c = getopt(argc, argv, "l:o:v")) != EOF) {
 		switch (c) {
 		case 'l':
-			length = cvtnum(fsblocksize, fssectsize, optarg);
+			length = io_cvtnum(optarg);
 			break;
 		case 'o':
-			offset = cvtnum(fsblocksize, fssectsize, optarg);
+			offset = io_cvtnum(optarg);
 			break;
 		case 'v':
 			verbose = 1;
diff --git a/io/reflink.c b/io/reflink.c
index f584e8f..ba08152 100644
--- a/io/reflink.c
+++ b/io/reflink.c
@@ -113,12 +113,10 @@ dedupe_f(
 	long long	count, total;
 	char		*infile;
 	int		condensed, quiet_flag;
-	size_t		fsblocksize, fssectsize;
 	struct timeval	t1, t2;
 	int		c, ops = 0, fd = -1;
 
 	condensed = quiet_flag = 0;
-	init_cvtnum(&fsblocksize, &fssectsize);
 
 	while ((c = getopt(argc, argv, "Cq")) != EOF) {
 		switch (c) {
@@ -136,19 +134,19 @@ dedupe_f(
 		return command_usage(&dedupe_cmd);
 	infile = argv[optind];
 	optind++;
-	soffset = cvtnum(fsblocksize, fssectsize, argv[optind]);
+	soffset = io_cvtnum(argv[optind]);
 	if (soffset < 0) {
 		printf(_("non-numeric src offset argument -- %s\n"), argv[optind]);
 		return 0;
 	}
 	optind++;
-	doffset = cvtnum(fsblocksize, fssectsize, argv[optind]);
+	doffset = io_cvtnum(argv[optind]);
 	if (doffset < 0) {
 		printf(_("non-numeric dest offset argument -- %s\n"), argv[optind]);
 		return 0;
 	}
 	optind++;
-	count = cvtnum(fsblocksize, fssectsize, argv[optind]);
+	count = io_cvtnum(argv[optind]);
 	if (count < 0) {
 		printf(_("non-positive length argument -- %s\n"), argv[optind]);
 		return 0;
@@ -233,13 +231,11 @@ reflink_f(
 	long long	count = 0, total;
 	char		*infile = NULL;
 	int		condensed, quiet_flag;
-	size_t		fsblocksize, fssectsize;
 	struct timeval	t1, t2;
 	int		c, ops = 0, fd = -1;
 
 	condensed = quiet_flag = 0;
 	doffset = soffset = 0;
-	init_cvtnum(&fsblocksize, &fssectsize);
 
 	while ((c = getopt(argc, argv, "Cq")) != EOF) {
 		switch (c) {
@@ -259,19 +255,19 @@ reflink_f(
 	optind++;
 	if (optind == argc)
 		goto clone_all;
-	soffset = cvtnum(fsblocksize, fssectsize, argv[optind]);
+	soffset = io_cvtnum(argv[optind]);
 	if (soffset < 0) {
 		printf(_("non-numeric src offset argument -- %s\n"), argv[optind]);
 		return 0;
 	}
 	optind++;
-	doffset = cvtnum(fsblocksize, fssectsize, argv[optind]);
+	doffset = io_cvtnum(argv[optind]);
 	if (doffset < 0) {
 		printf(_("non-numeric dest offset argument -- %s\n"), argv[optind]);
 		return 0;
 	}
 	optind++;
-	count = cvtnum(fsblocksize, fssectsize, argv[optind]);
+	count = io_cvtnum(argv[optind]);
 	if (count < 0) {
 		printf(_("non-positive length argument -- %s\n"), argv[optind]);
 		return 0;
diff --git a/io/resblks.c b/io/resblks.c
index 06903f5..337ea32 100644
--- a/io/resblks.c
+++ b/io/resblks.c
@@ -33,7 +33,7 @@ resblks_f(
 	long long		blks;
 
 	if (argc == 2) {
-		blks = cvtnum(file->geom.blocksize, file->geom.sectsize, argv[1]);
+		blks = io_cvtnum(argv[1]);
 		if (blks < 0) {
 			printf(_("non-numeric argument -- %s\n"), argv[1]);
 			return 0;
diff --git a/io/seek.c b/io/seek.c
index 871b472..51c070f 100644
--- a/io/seek.c
+++ b/io/seek.c
@@ -105,14 +105,12 @@ seek_f(
 	char	**argv)
 {
 	off64_t		offset, start;
-	size_t		fsblocksize, fssectsize;
 	int		c;
 	int		current;	/* specify data or hole */
 	int		flag;
 	int		startflag;
 
 	flag = startflag = 0;
-	init_cvtnum(&fsblocksize, &fssectsize);
 
 	while ((c = getopt(argc, argv, "adhrs")) != EOF) {
 		switch (c) {
@@ -138,7 +136,7 @@ seek_f(
 	if (!(flag & (SEEK_DFLAG | SEEK_HFLAG)) || optind != argc - 1)
 		return command_usage(&seek_cmd);
 
-	start = offset = cvtnum(fsblocksize, fssectsize, argv[optind]);
+	start = offset = io_cvtnum(argv[optind]);
 	if (offset < 0)
 		return command_usage(&seek_cmd);
 
diff --git a/io/sendfile.c b/io/sendfile.c
index 063fa7f..c03b422 100644
--- a/io/sendfile.c
+++ b/io/sendfile.c
@@ -79,14 +79,12 @@ sendfile_f(
 {
 	off64_t		offset = 0;
 	long long	count, total;
-	size_t		blocksize, sectsize;
 	struct timeval	t1, t2;
 	char		*infile = NULL;
 	int		Cflag, qflag;
 	int		c, fd = -1;
 
 	Cflag = qflag = 0;
-	init_cvtnum(&blocksize, &sectsize);
 	while ((c = getopt(argc, argv, "Cf:i:q")) != EOF) {
 		switch (c) {
 		case 'C':
@@ -119,14 +117,14 @@ sendfile_f(
 		return 0;
 
 	if (optind == argc - 2) {
-		offset = cvtnum(blocksize, sectsize, argv[optind]);
+		offset = io_cvtnum(argv[optind]);
 		if (offset < 0) {
 			printf(_("non-numeric offset argument -- %s\n"),
 				argv[optind]);
 			goto done;
 		}
 		optind++;
-		count = cvtnum(blocksize, sectsize, argv[optind]);
+		count = io_cvtnum(argv[optind]);
 		if (count < 0) {
 			printf(_("non-numeric length argument -- %s\n"),
 				argv[optind]);
diff --git a/io/sync_file_range.c b/io/sync_file_range.c
index 7e4f3e6..6634032 100644
--- a/io/sync_file_range.c
+++ b/io/sync_file_range.c
@@ -44,7 +44,6 @@ sync_range_f(
 {
 	off64_t		offset = 0, length = 0;
 	int		c, sync_mode = 0;
-	size_t		blocksize, sectsize;
 
 	while ((c = getopt(argc, argv, "abw")) != EOF) {
 		switch (c) {
@@ -68,15 +67,14 @@ sync_range_f(
 
 	if (optind != argc - 2)
 		return command_usage(&sync_range_cmd);
-	init_cvtnum(&blocksize, &sectsize);
-	offset = cvtnum(blocksize, sectsize, argv[optind]);
+	offset = io_cvtnum(argv[optind]);
 	if (offset < 0) {
 		printf(_("non-numeric offset argument -- %s\n"),
 			argv[optind]);
 		return 0;
 	}
 	optind++;
-	length = cvtnum(blocksize, sectsize, argv[optind]);
+	length = io_cvtnum(argv[optind]);
 	if (length < 0) {
 		printf(_("non-numeric length argument -- %s\n"),
 			argv[optind]);
diff --git a/io/truncate.c b/io/truncate.c
index 20bada8..abfee9f 100644
--- a/io/truncate.c
+++ b/io/truncate.c
@@ -29,10 +29,8 @@ truncate_f(
 	char		**argv)
 {
 	off64_t		offset;
-	size_t		blocksize, sectsize;
 
-	init_cvtnum(&blocksize, &sectsize);
-	offset = cvtnum(blocksize, sectsize, argv[1]);
+	offset = io_cvtnum(argv[1]);
 	if (offset < 0) {
 		printf(_("non-numeric truncate argument -- %s\n"), argv[1]);
 		return 0;

[PATCH 4/3] xfs_quota: remove dodgy init_cvtnum

[Eric Sandeen]

init_cvtnum() in quota is a lie; it sets blocksize to
4096 and sector size to 512 unconditionally.

Now that cvtnum can cope with unset block and/or sectors,
just pass in zeros, meaning that we can't specify limits
in terms of block or sector count.

Now that we're not using the "fake" sizes, remove them
from the extractb() args as well.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/quota/edit.c b/quota/edit.c
index 8418e85..aa8c14e 100644
--- a/quota/edit.c
+++ b/quota/edit.c
@@ -222,8 +222,6 @@ extractb(
 	char		*string,
 	const char	*prefix,
 	int		length,
-	uint		blocksize,
-	uint		sectorsize,
 	uint64_t	*value)
 {
 	long long	v;
@@ -231,7 +229,7 @@ extractb(
 
 	if (strncmp(string, prefix, length) == 0) {
 		s = string + length + 1;
-		v = cvtnum(blocksize, sectorsize, s);
+		v = cvtnum(0, 0, s);
 		if (v == -1LL) {
 			fprintf(stderr,
 				_("%s: Error: could not parse size %s.\n"),
@@ -272,9 +270,8 @@ limit_f(
 	char		*name;
 	uint64_t	bsoft, bhard, isoft, ihard, rtbsoft, rtbhard;
 	int		c, type = 0, mask = 0, flags = 0;
-	uint		bsize, ssize, endoptions;
+	uint		endoptions;
 
-	init_cvtnum(&bsize, &ssize);
 	bsoft = bhard = isoft = ihard = rtbsoft = rtbhard = 0;
 	while ((c = getopt(argc, argv, "dgpu")) != EOF) {
 		switch (c) {
@@ -315,17 +312,17 @@ limit_f(
 	 */
 	while (argc > optind + endoptions - 1) {
 		char *s = argv[optind++];
-		if (extractb(s, "bsoft=", 5, bsize, ssize, &bsoft))
+		if (extractb(s, "bsoft=", 5, &bsoft))
 			mask |= FS_DQ_BSOFT;
-		else if (extractb(s, "bhard=", 5, bsize, ssize, &bhard))
+		else if (extractb(s, "bhard=", 5, &bhard))
 			mask |= FS_DQ_BHARD;
 		else if (extracti(s, "isoft=", 5, &isoft))
 			mask |= FS_DQ_ISOFT;
 		else if (extracti(s, "ihard=", 5, &ihard))
 			mask |= FS_DQ_IHARD;
-		else if (extractb(s, "rtbsoft=", 7, bsize, ssize, &rtbsoft))
+		else if (extractb(s, "rtbsoft=", 7, &rtbsoft))
 			mask |= FS_DQ_RTBSOFT;
-		else if (extractb(s, "rtbhard=", 7, bsize, ssize, &rtbhard))
+		else if (extractb(s, "rtbhard=", 7, &rtbhard))
 			mask |= FS_DQ_RTBHARD;
 		else {
 			exitcode = 1;
diff --git a/quota/init.c b/quota/init.c
index d45dc4c..eb2b9ca 100644
--- a/quota/init.c
+++ b/quota/init.c
@@ -51,15 +51,6 @@ usage(void)
 	exit(1);
 }
 
-void
-init_cvtnum(
-	unsigned int	*blocksize,
-	unsigned int	*sectsize)
-{
-	*blocksize = 4096;
-	*sectsize = 512;
-}
-
 static void
 init_commands(void)
 {
diff --git a/quota/init.h b/quota/init.h
index 6879855..10b4d68 100644
--- a/quota/init.h
+++ b/quota/init.h
@@ -29,5 +29,3 @@ extern void	quot_init(void);
 extern void	quota_init(void);
 extern void	report_init(void);
 extern void	state_init(void);
-
-extern void init_cvtnum(unsigned int *, unsigned int *);

Re: [PATCH 1/3] libxfs: handle 0 blocksize or sectorsize in cvtnum

[Dave Chinner]

On Tue, Aug 22, 2017 at 04:12:48PM -0500, Eric Sandeen wrote:
> Blocksize and sectorsize are unique in that they must
> be provided, unlike every other suffix which can be
> calculated from constants.
> 
> Nothing protects against unspecified block & sector size,
> so catch it if it happens and return a parsing error.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/libxcmd/input.c b/libxcmd/input.c
> index 7a69dc1..7b86225 100644
> --- a/libxcmd/input.c
> +++ b/libxcmd/input.c
> @@ -330,8 +330,12 @@ cvtnum(
>  	c = tolower(*sp);
>  	switch (c) {
>  	case 'b':
> +		if (!blocksize)
> +			return -1LL;
>  		return i * blocksize;
>  	case 's':
> +		if (!sectorsize)
> +			return -1LL;
>  		return i * sectorsize;
>  	case 'k':
>  		return KILOBYTES(i);

With this you could have mkfs call the generic function, too.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/3] libxfs: handle 0 blocksize or sectorsize in cvtnum

[Eric Sandeen]

On 8/22/17 8:01 PM, Dave Chinner wrote:
> On Tue, Aug 22, 2017 at 04:12:48PM -0500, Eric Sandeen wrote:
>> Blocksize and sectorsize are unique in that they must
>> be provided, unlike every other suffix which can be
>> calculated from constants.
>>
>> Nothing protects against unspecified block & sector size,
>> so catch it if it happens and return a parsing error.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/libxcmd/input.c b/libxcmd/input.c
>> index 7a69dc1..7b86225 100644
>> --- a/libxcmd/input.c
>> +++ b/libxcmd/input.c
>> @@ -330,8 +330,12 @@ cvtnum(
>>  	c = tolower(*sp);
>>  	switch (c) {
>>  	case 'b':
>> +		if (!blocksize)
>> +			return -1LL;
>>  		return i * blocksize;
>>  	case 's':
>> +		if (!sectorsize)
>> +			return -1LL;
>>  		return i * sectorsize;
>>  	case 'k':
>>  		return KILOBYTES(i);
> 
> With this you could have mkfs call the generic function, too.

Yep, or at least closer; mkfs does a printf and usage()/exit()
and it indicates a user error (most likely)

in libxcmd/xfs_io it's more of a programming error, with no 
printf to the user at least today... so would need a little more
finessing.

-Eric

Re: [PATCH 1/3] libxfs: handle 0 blocksize or sectorsize in cvtnum

[Darrick J. Wong]

On Tue, Aug 22, 2017 at 09:13:03PM -0500, Eric Sandeen wrote:
> On 8/22/17 8:01 PM, Dave Chinner wrote:
> > On Tue, Aug 22, 2017 at 04:12:48PM -0500, Eric Sandeen wrote:
> >> Blocksize and sectorsize are unique in that they must
> >> be provided, unlike every other suffix which can be
> >> calculated from constants.
> >>
> >> Nothing protects against unspecified block & sector size,
> >> so catch it if it happens and return a parsing error.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> ---
> >>
> >> diff --git a/libxcmd/input.c b/libxcmd/input.c
> >> index 7a69dc1..7b86225 100644
> >> --- a/libxcmd/input.c
> >> +++ b/libxcmd/input.c
> >> @@ -330,8 +330,12 @@ cvtnum(
> >>  	c = tolower(*sp);
> >>  	switch (c) {
> >>  	case 'b':
> >> +		if (!blocksize)
> >> +			return -1LL;
> >>  		return i * blocksize;
> >>  	case 's':
> >> +		if (!sectorsize)
> >> +			return -1LL;
> >>  		return i * sectorsize;
> >>  	case 'k':
> >>  		return KILOBYTES(i);
> > 
> > With this you could have mkfs call the generic function, too.
> 
> Yep, or at least closer; mkfs does a printf and usage()/exit()
> and it indicates a user error (most likely)
> 
> in libxcmd/xfs_io it's more of a programming error, with no 
> printf to the user at least today... so would need a little more
> finessing.

This series looks mostly fine to me, but I'm wondering if io_cvtnum
should be promoted so that mkfs/spaceman/quota/etc can take advantage of
it too?

--D

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

Re: [PATCH 1/3] libxfs: handle 0 blocksize or sectorsize in cvtnum

[Eric Sandeen]

On 8/23/17 3:14 PM, Darrick J. Wong wrote:
> On Tue, Aug 22, 2017 at 09:13:03PM -0500, Eric Sandeen wrote:
>> On 8/22/17 8:01 PM, Dave Chinner wrote:
>>> On Tue, Aug 22, 2017 at 04:12:48PM -0500, Eric Sandeen wrote:
>>>> Blocksize and sectorsize are unique in that they must
>>>> be provided, unlike every other suffix which can be
>>>> calculated from constants.
>>>>
>>>> Nothing protects against unspecified block & sector size,
>>>> so catch it if it happens and return a parsing error.
>>>>
>>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>>> ---
>>>>
>>>> diff --git a/libxcmd/input.c b/libxcmd/input.c
>>>> index 7a69dc1..7b86225 100644
>>>> --- a/libxcmd/input.c
>>>> +++ b/libxcmd/input.c
>>>> @@ -330,8 +330,12 @@ cvtnum(
>>>>  	c = tolower(*sp);
>>>>  	switch (c) {
>>>>  	case 'b':
>>>> +		if (!blocksize)
>>>> +			return -1LL;
>>>>  		return i * blocksize;
>>>>  	case 's':
>>>> +		if (!sectorsize)
>>>> +			return -1LL;
>>>>  		return i * sectorsize;
>>>>  	case 'k':
>>>>  		return KILOBYTES(i);
>>>
>>> With this you could have mkfs call the generic function, too.
>>
>> Yep, or at least closer; mkfs does a printf and usage()/exit()
>> and it indicates a user error (most likely)
>>
>> in libxcmd/xfs_io it's more of a programming error, with no 
>> printf to the user at least today... so would need a little more
>> finessing.
> 
> This series looks mostly fine to me, but I'm wondering if io_cvtnum
> should be promoted so that mkfs/spaceman/quota/etc can take advantage of
> it too?

I thought about that, but it relies on the global var "file" that io uses,
which seems a bit unique to io, no?

-Eric

Re: [PATCH 1/3] libxfs: handle 0 blocksize or sectorsize in cvtnum

[Darrick J. Wong]

On Wed, Aug 23, 2017 at 03:18:45PM -0500, Eric Sandeen wrote:
> 
> 
> On 8/23/17 3:14 PM, Darrick J. Wong wrote:
> > On Tue, Aug 22, 2017 at 09:13:03PM -0500, Eric Sandeen wrote:
> >> On 8/22/17 8:01 PM, Dave Chinner wrote:
> >>> On Tue, Aug 22, 2017 at 04:12:48PM -0500, Eric Sandeen wrote:
> >>>> Blocksize and sectorsize are unique in that they must
> >>>> be provided, unlike every other suffix which can be
> >>>> calculated from constants.
> >>>>
> >>>> Nothing protects against unspecified block & sector size,
> >>>> so catch it if it happens and return a parsing error.
> >>>>
> >>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >>>> ---
> >>>>
> >>>> diff --git a/libxcmd/input.c b/libxcmd/input.c
> >>>> index 7a69dc1..7b86225 100644
> >>>> --- a/libxcmd/input.c
> >>>> +++ b/libxcmd/input.c
> >>>> @@ -330,8 +330,12 @@ cvtnum(
> >>>>  	c = tolower(*sp);
> >>>>  	switch (c) {
> >>>>  	case 'b':
> >>>> +		if (!blocksize)
> >>>> +			return -1LL;
> >>>>  		return i * blocksize;
> >>>>  	case 's':
> >>>> +		if (!sectorsize)
> >>>> +			return -1LL;
> >>>>  		return i * sectorsize;
> >>>>  	case 'k':
> >>>>  		return KILOBYTES(i);
> >>>
> >>> With this you could have mkfs call the generic function, too.
> >>
> >> Yep, or at least closer; mkfs does a printf and usage()/exit()
> >> and it indicates a user error (most likely)
> >>
> >> in libxcmd/xfs_io it's more of a programming error, with no 
> >> printf to the user at least today... so would need a little more
> >> finessing.
> > 
> > This series looks mostly fine to me, but I'm wondering if io_cvtnum
> > should be promoted so that mkfs/spaceman/quota/etc can take advantage of
> > it too?
> 
> I thought about that, but it relies on the global var "file" that io uses,
> which seems a bit unique to io, no?

Spaceman has it too.

I guess quota does its own weird thing...

--D

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