* [PATCH 1/5] Move parse_size() to utils.[hc]
2012-10-23 17:51 [PATCH][BTRFS-PROGS][V2] Update to parse_size() Goffredo Baroncelli
@ 2012-10-23 17:51 ` Goffredo Baroncelli
2012-10-23 17:51 ` [PATCH 2/5] parse_size(): replace atoll() with strtoull() Goffredo Baroncelli
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Goffredo Baroncelli @ 2012-10-23 17:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: Stefan Behrens, Wang Sheng-Hui, Goffredo Baroncelli
From: Goffredo Baroncelli <kreijack@inwind.it>
Move the function from cmds-filesystem.c and mkfs.c to utils.c
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/5] parse_size(): replace atoll() with strtoull()
2012-10-23 17:51 [PATCH][BTRFS-PROGS][V2] Update to parse_size() Goffredo Baroncelli
2012-10-23 17:51 ` [PATCH 1/5] Move parse_size() to utils.[hc] Goffredo Baroncelli
@ 2012-10-23 17:51 ` Goffredo Baroncelli
2012-10-23 17:51 ` [PATCH 3/5] parse_size(): check for invalid suffix Goffredo Baroncelli
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Goffredo Baroncelli @ 2012-10-23 17:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: Stefan Behrens, Wang Sheng-Hui, Goffredo Baroncelli
From: Goffredo Baroncelli <kreijack@inwind.it>
Replace the function atoll with strtoull()
---
utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/utils.c b/utils.c
index 705be7b..732c782 100644
--- a/utils.c
+++ b/utils.c
@@ -1243,6 +1243,6 @@ u64 parse_size(char *s)
}
s[len - 1] = '\0';
}
- return atoll(s) * mult;
+ return strtoull(s, NULL, 0) * mult;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/5] parse_size(): check for invalid suffix
2012-10-23 17:51 [PATCH][BTRFS-PROGS][V2] Update to parse_size() Goffredo Baroncelli
2012-10-23 17:51 ` [PATCH 1/5] Move parse_size() to utils.[hc] Goffredo Baroncelli
2012-10-23 17:51 ` [PATCH 2/5] parse_size(): replace atoll() with strtoull() Goffredo Baroncelli
@ 2012-10-23 17:51 ` Goffredo Baroncelli
2012-10-23 17:51 ` [PATCH 4/5] parse_size(): add new suffixes Goffredo Baroncelli
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Goffredo Baroncelli @ 2012-10-23 17:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: Stefan Behrens, Wang Sheng-Hui, Goffredo Baroncelli
From: Goffredo Baroncelli <kreijack@inwind.it>
Check that the suffix for the parse_size() input is of only
one character.
---
utils.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/utils.c b/utils.c
index 732c782..e61cdea 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 strtoull(s, NULL, 0) * 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/5] parse_size(): add new suffixes
2012-10-23 17:51 [PATCH][BTRFS-PROGS][V2] Update to parse_size() Goffredo Baroncelli
` (2 preceding siblings ...)
2012-10-23 17:51 ` [PATCH 3/5] parse_size(): check for invalid suffix Goffredo Baroncelli
@ 2012-10-23 17:51 ` Goffredo Baroncelli
2012-10-23 17:51 ` [PATCH 5/5] Update the man page with the new prefixes Goffredo Baroncelli
2012-10-23 18:48 ` [PATCH][BTRFS-PROGS][V2] Update to parse_size() Goffredo Baroncelli
5 siblings, 0 replies; 10+ messages in thread
From: Goffredo Baroncelli @ 2012-10-23 17:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: Stefan Behrens, Wang Sheng-Hui, Goffredo Baroncelli
From: Goffredo Baroncelli <kreijack@inwind.it>
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 .
---
utils.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/utils.c b/utils.c
index e61cdea..4b16fc9 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/5] Update the man page with the new prefixes.
2012-10-23 17:51 [PATCH][BTRFS-PROGS][V2] Update to parse_size() Goffredo Baroncelli
` (3 preceding siblings ...)
2012-10-23 17:51 ` [PATCH 4/5] parse_size(): add new suffixes Goffredo Baroncelli
@ 2012-10-23 17:51 ` Goffredo Baroncelli
2012-10-25 16:13 ` David Sterba
2012-10-23 18:48 ` [PATCH][BTRFS-PROGS][V2] Update to parse_size() Goffredo Baroncelli
5 siblings, 1 reply; 10+ messages in thread
From: Goffredo Baroncelli @ 2012-10-23 17:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: Stefan Behrens, Wang Sheng-Hui, Goffredo Baroncelli
From: 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..33bd78d 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 prefix
+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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 5/5] Update the man page with the new prefixes.
2012-10-23 17:51 ` [PATCH 5/5] Update the man page with the new prefixes Goffredo Baroncelli
@ 2012-10-25 16:13 ` David Sterba
2012-10-25 19:22 ` Goffredo Baroncelli
0 siblings, 1 reply; 10+ messages in thread
From: David Sterba @ 2012-10-25 16:13 UTC (permalink / raw)
To: Goffredo Baroncelli
Cc: linux-btrfs, Stefan Behrens, Wang Sheng-Hui, Goffredo Baroncelli
Hi,
one minor typo
On Tue, Oct 23, 2012 at 07:51:55PM +0200, Goffredo Baroncelli wrote:
> --- 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 prefix
^^^^^^
suffix
> +like \fBk\fP for 1 KBytes, \fBm\fP for 1 MBytes...
> +
david
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 5/5] Update the man page with the new prefixes.
2012-10-25 16:13 ` David Sterba
@ 2012-10-25 19:22 ` Goffredo Baroncelli
0 siblings, 0 replies; 10+ messages in thread
From: Goffredo Baroncelli @ 2012-10-25 19:22 UTC (permalink / raw)
To: Goffredo Baroncelli, linux-btrfs, Stefan Behrens, Wang Sheng-Hui
On 2012-10-25 18:13, David Sterba wrote:
> Hi,
>
> one minor typo
>
> On Tue, Oct 23, 2012 at 07:51:55PM +0200, Goffredo Baroncelli wrote:
>> --- 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 prefix
> ^^^^^^
> suffix
Thanks, this week I will update the patches.
>
>> +like \fBk\fP for 1 KBytes, \fBm\fP for 1 MBytes...
>> +
>
> david
> .
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][BTRFS-PROGS][V2] Update to parse_size()
2012-10-23 17:51 [PATCH][BTRFS-PROGS][V2] Update to parse_size() Goffredo Baroncelli
` (4 preceding siblings ...)
2012-10-23 17:51 ` [PATCH 5/5] Update the man page with the new prefixes Goffredo Baroncelli
@ 2012-10-23 18:48 ` Goffredo Baroncelli
2012-10-25 16:33 ` David Sterba
5 siblings, 1 reply; 10+ messages in thread
From: Goffredo Baroncelli @ 2012-10-23 18:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: Stefan Behrens, Wang Sheng-Hui
Sorry the version is V1 and not V2
On Tue, Oct 23, 2012 at 7:51 PM, Goffredo Baroncelli <kreijack@gmail.com> wrote:
> 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 <kreij...@inwind.it>
>
> Changelog:
> V1: avoid to change the parse_size argument;
> better check of a wrong suffix;
> force strtoull to use a decimal base
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH][BTRFS-PROGS][V2] Update to parse_size()
2012-10-23 18:48 ` [PATCH][BTRFS-PROGS][V2] Update to parse_size() Goffredo Baroncelli
@ 2012-10-25 16:33 ` David Sterba
0 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2012-10-25 16:33 UTC (permalink / raw)
To: Goffredo Baroncelli; +Cc: linux-btrfs, Stefan Behrens, Wang Sheng-Hui
On Tue, Oct 23, 2012 at 08:48:08PM +0200, Goffredo Baroncelli wrote:
> Sorry the version is V1 and not V2
> > Changelog:
> > V1: avoid to change the parse_size argument;
> > better check of a wrong suffix;
> > force strtoull to use a decimal base
[I looked at patches at your git repo, not sure if this was the V2, but
the changelog seems to apply]
Code basically looks ok, the individual patches do not have your
Signed-off-by lines, please add them for the final version. There are a
few code style issues, like
+ for( i=0 ; s[i] && isdigit(s[i]) ; i++ ) ;
+ if( !i ){
ie. the spaces around keywords etc. Progs share lots of code from kernel
and thus this is the prevalent stylep, please keep it like that (unless
the maintainer is fine with your style of course :)
You can drop the patch "Replace the function atoll with strtoull()" and
fold the change into "Check that the suffix for the parse_size() input
is of only one character."
> On Tue, Oct 23, 2012 at 7:51 PM, Goffredo Baroncelli <kreijack@gmail.com> wrote:
> > 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()
Nice set of updates, thanks.
david
^ permalink raw reply [flat|nested] 10+ messages in thread