Hi all,
the following patches attempt to address some issues to the function
parse_size():
- this function is defined both in mkfs.c and cmds-filesystem.c; I
moved it in utils.c (which is already used in both mkfs.btrfs and
btrfs) in order to avoid code duplication.
- it used the function atoll(); I replaceed atoll() with strtoull()
because we are dealing with u64
- no check on suffixes was performed. If the user put ''MB'' as
suffix he got
bytes instead megabytes. The patches check the suffix is valid
- add new suffixes (t,p,e for terabytes, petabytes, exabytes)
- update the man page of the command mkfs.btrfs and
"btrfs filesystem defragment", both use parse_size()
Several peoples (see cc''s) suggested these improvements with different
patches, I collected them togheter.
Please reviewed them, test them. Comments are welcome.
The patches are available also to
You can pull the patch from
http://cassiopea.homelinux.net/git/btrfs-progs-unstable.git
branch
parse_size
Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
Changelog:
V3: Corrected few code style issues; replace a wrong "prefix" with
suffix in
the man page. Added Signed-off-by in every patch.
V2 -> skipped due to a worng subject in previous emails
V1: avoid to change the parse_size argument;
better check of a wrong suffix;
force strtoull to use a decimal base
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Move the function from cmds-filesystem.c and mkfs.c to utils.c
Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
cmds-filesystem.c | 26 --------------------------
mkfs.c | 31 -------------------------------
utils.c | 26 ++++++++++++++++++++++++++
utils.h | 2 ++
4 files changed, 28 insertions(+), 57 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 9c43d35..507239a 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -311,32 +311,6 @@ static int cmd_sync(int argc, char **argv)
return 0;
}
-static u64 parse_size(char *s)
-{
- int len = strlen(s);
- char c;
- u64 mult = 1;
-
- if (!isdigit(s[len - 1])) {
- c = tolower(s[len - 1]);
- switch (c) {
- case ''g'':
- mult *= 1024;
- case ''m'':
- mult *= 1024;
- case ''k'':
- mult *= 1024;
- case ''b'':
- break;
- default:
- fprintf(stderr, "Unknown size descriptor %c\n", c);
- exit(1);
- }
- s[len - 1] = ''\0'';
- }
- return atoll(s) * mult;
-}
-
static int parse_compress_type(char *s)
{
if (strcmp(optarg, "zlib") == 0)
diff --git a/mkfs.c b/mkfs.c
index 47f0c9c..ca850d9 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -54,37 +54,6 @@ struct directory_name_entry {
struct list_head list;
};
-static u64 parse_size(char *s)
-{
- int len = strlen(s);
- char c;
- u64 mult = 1;
- u64 ret;
-
- s = strdup(s);
-
- if (len && !isdigit(s[len - 1])) {
- c = tolower(s[len - 1]);
- switch (c) {
- case ''g'':
- mult *= 1024;
- case ''m'':
- mult *= 1024;
- case ''k'':
- mult *= 1024;
- case ''b'':
- break;
- default:
- fprintf(stderr, "Unknown size descriptor %c\n", c);
- exit(1);
- }
- s[len - 1] = ''\0'';
- }
- ret = atol(s) * mult;
- free(s);
- return ret;
-}
-
static int make_root_dir(struct btrfs_root *root, int mixed)
{
struct btrfs_trans_handle *trans;
diff --git a/utils.c b/utils.c
index 205e667..705be7b 100644
--- a/utils.c
+++ b/utils.c
@@ -1220,3 +1220,29 @@ scan_again:
return 0;
}
+u64 parse_size(char *s)
+{
+ int len = strlen(s);
+ char c;
+ u64 mult = 1;
+
+ if (!isdigit(s[len - 1])) {
+ c = tolower(s[len - 1]);
+ switch (c) {
+ case ''g'':
+ mult *= 1024;
+ case ''m'':
+ mult *= 1024;
+ case ''k'':
+ mult *= 1024;
+ case ''b'':
+ break;
+ default:
+ fprintf(stderr, "Unknown size descriptor %c\n", c);
+ exit(1);
+ }
+ s[len - 1] = ''\0'';
+ }
+ return atoll(s) * mult;
+}
+
diff --git a/utils.h b/utils.h
index 3a0368b..714fd7a 100644
--- a/utils.h
+++ b/utils.h
@@ -46,4 +46,6 @@ int check_label(char *input);
int get_mountpt(char *dev, char *mntpt, size_t size);
int btrfs_scan_block_devices(int run_ioctl);
+
+u64 parse_size(char *s);
#endif
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Goffredo Baroncelli
2012-Oct-29 17:53 UTC
[PATCH 2/4] parse_size(): replace atoll() with strtoull()
Replace the function atoll with strtoull(); Check that the suffix for the
parse_size() input is of only one character.
Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
utils.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/utils.c b/utils.c
index 705be7b..aa2e574 100644
--- a/utils.c
+++ b/utils.c
@@ -31,6 +31,7 @@
#include <fcntl.h>
#include <unistd.h>
#include <mntent.h>
+#include <ctype.h>
#include <linux/loop.h>
#include <linux/major.h>
#include <linux/kdev_t.h>
@@ -1222,12 +1223,18 @@ scan_again:
u64 parse_size(char *s)
{
- int len = strlen(s);
+ int i;
char c;
u64 mult = 1;
- if (!isdigit(s[len - 1])) {
- c = tolower(s[len - 1]);
+ for (i=0 ; s[i] && isdigit(s[i]) ; i++) ;
+ if (!i) {
+ fprintf(stderr, "ERROR: size value is empty\n");
+ exit(50);
+ }
+
+ if (s[i]) {
+ c = tolower(s[i]);
switch (c) {
case ''g'':
mult *= 1024;
@@ -1238,11 +1245,17 @@ u64 parse_size(char *s)
case ''b'':
break;
default:
- fprintf(stderr, "Unknown size descriptor %c\n", c);
+ fprintf(stderr, "ERROR: Unknown size descriptor "
+ "''%c''\n", c);
exit(1);
}
- s[len - 1] = ''\0'';
}
- return atoll(s) * mult;
+ if (s[i] && s[i+1]) {
+ fprintf(stderr, "ERROR: Illegal suffix contains "
+ "character ''%c'' in wrong position\n",
+ s[i+1]);
+ exit(51);
+ }
+ return strtoull(s, NULL, 10) * mult;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Add new suffixes in parse_size() function. New suffixes are: T as
terabyte, P as petabyte, E as exabyte. Note these units are
multiply of 2 .
Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
utils.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/utils.c b/utils.c
index aa2e574..b487be6 100644
--- a/utils.c
+++ b/utils.c
@@ -1236,6 +1236,12 @@ u64 parse_size(char *s)
if (s[i]) {
c = tolower(s[i]);
switch (c) {
+ case ''e'':
+ mult *= 1024;
+ case ''p'':
+ mult *= 1024;
+ case ''t'':
+ mult *= 1024;
case ''g'':
mult *= 1024;
case ''m'':
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Goffredo Baroncelli
2012-Oct-29 17:53 UTC
[PATCH 4/4] Update the man page with the new prefixes.
Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it> --- man/btrfs.8.in | 3 +++ man/mkfs.btrfs.8.in | 3 +++ 2 files changed, 6 insertions(+) diff --git a/man/btrfs.8.in b/man/btrfs.8.in index 9222580..3564e6a 100644 --- a/man/btrfs.8.in +++ b/man/btrfs.8.in @@ -184,6 +184,9 @@ defragment operations. \fB-t size\fP defragment only files at least \fIsize\fR bytes big +For \fBstart\fP, \fBlen\fP, \fBsize\fP it is possible to append a suffix +like \fBk\fP for 1 KBytes, \fBm\fP for 1 MBytes... + NOTE: defragmenting with kernels up to 2.6.37 will unlink COW-ed copies of data, don''t use it if you use snapshots, have de-duplicated your data or made copies with \fBcp --reflink\fP. diff --git a/man/mkfs.btrfs.8.in b/man/mkfs.btrfs.8.in index 72025ed..6f37cd8 100644 --- a/man/mkfs.btrfs.8.in +++ b/man/mkfs.btrfs.8.in @@ -69,6 +69,9 @@ Do not perform whole device TRIM operation by default. .TP \fB\-V\fR, \fB\-\-version\fR Print the \fBmkfs.btrfs\fP version and exit. +.SH UNIT +As default the unit is the byte, however it is possible to append a suffix +to the arguments like \fBk\fP for KBytes, \fBm\fP for MBytes... .SH AVAILABILITY .B mkfs.btrfs is part of btrfs-progs. Btrfs is currently under heavy development, -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html