* [PATCH v2 1/4] Btrfs-progs: new helper to parse string to u64 for btrfs
2014-02-20 1:30 [PATCH v2 0/4] Btrfs-progs: cleanups: new helper for parsing string to u64 Wang Shilong
@ 2014-02-20 1:30 ` Wang Shilong
2014-02-20 1:39 ` Eric Sandeen
2014-02-20 1:30 ` [PATCH v2 2/4] Btrfs-progs: switch to arg_strtou64() part1 Wang Shilong
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Wang Shilong @ 2014-02-20 1:30 UTC (permalink / raw)
To: linux-btrfs
There are many places that need parse string to u64 for btrfs commands,
in fact, we do such things *too casually*, using atoi/atol/atoll..is not
right at all, and even we don't check whether it is a valid string.
Let's do everything more gracefully, we introduce a new helper
arg_strtou64() which will do all the necessary checks.If we fail to
parse string to u64, we will output message and exit directly, this is
something like what usage() is doing. It is ok to not return erro to
it's caller, because this function should be called when parsing arg
(just like usage!)
Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
utils.c | 33 +++++++++++++++++++++++++++++++++
utils.h | 1 +
2 files changed, 34 insertions(+)
diff --git a/utils.c b/utils.c
index 97e23d5..d570967 100644
--- a/utils.c
+++ b/utils.c
@@ -1520,6 +1520,39 @@ scan_again:
return 0;
}
+/*
+ * This function should be only used when parsing
+ * command arg, it won't return error to it's
+ * caller and rather exit directly just like usage().
+ */
+u64 arg_strtou64(const char *str)
+{
+ u64 value;
+ char *ptr_parse_end = NULL;
+ char *ptr_str_end = (char *)str + strlen(str);
+
+ /*
+ * if we pass a negative number to strtoull,
+ * it will return an unexpected number to us,
+ * so let's do the check ourselves firstly.
+ */
+ if (str[0] == '-') {
+ fprintf(stderr, "ERROR: %s may be negative value.\n", str);
+ exit(1);
+ }
+
+ value = strtoull(str, &ptr_parse_end, 0);
+ if (ptr_parse_end != ptr_str_end) {
+ fprintf(stderr, "ERROR: %s is not valid value.\n", str);
+ exit(1);
+ }
+ if (value == ULLONG_MAX) {
+ fprintf(stderr, "ERROR: %s is too large.\n", str);
+ exit(1);
+ }
+ return value;
+}
+
u64 parse_size(char *s)
{
int i;
diff --git a/utils.h b/utils.h
index 04b8c45..a201085 100644
--- a/utils.h
+++ b/utils.h
@@ -71,6 +71,7 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_bytes);
int get_mountpt(char *dev, char *mntpt, size_t size);
int btrfs_scan_block_devices(int run_ioctl);
u64 parse_size(char *s);
+u64 arg_strtou64(const char *str);
int open_file_or_dir(const char *fname, DIR **dirstream);
void close_file_or_dir(int fd, DIR *dirstream);
int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/4] Btrfs-progs: new helper to parse string to u64 for btrfs
2014-02-20 1:30 ` [PATCH v2 1/4] Btrfs-progs: new helper to parse string to u64 for btrfs Wang Shilong
@ 2014-02-20 1:39 ` Eric Sandeen
2014-02-20 1:43 ` Wang Shilong
0 siblings, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2014-02-20 1:39 UTC (permalink / raw)
To: Wang Shilong, linux-btrfs
On 2/19/14, 7:30 PM, Wang Shilong wrote:
> There are many places that need parse string to u64 for btrfs commands,
> in fact, we do such things *too casually*, using atoi/atol/atoll..is not
> right at all, and even we don't check whether it is a valid string.
>
> Let's do everything more gracefully, we introduce a new helper
> arg_strtou64() which will do all the necessary checks.If we fail to
> parse string to u64, we will output message and exit directly, this is
> something like what usage() is doing. It is ok to not return erro to
> it's caller, because this function should be called when parsing arg
> (just like usage!)
>
> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
> ---
> utils.c | 33 +++++++++++++++++++++++++++++++++
> utils.h | 1 +
> 2 files changed, 34 insertions(+)
>
> diff --git a/utils.c b/utils.c
> index 97e23d5..d570967 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1520,6 +1520,39 @@ scan_again:
> return 0;
> }
>
> +/*
> + * This function should be only used when parsing
> + * command arg, it won't return error to it's
> + * caller and rather exit directly just like usage().
> + */
> +u64 arg_strtou64(const char *str)
> +{
> + u64 value;
> + char *ptr_parse_end = NULL;
> + char *ptr_str_end = (char *)str + strlen(str);
> +
> + /*
> + * if we pass a negative number to strtoull,
> + * it will return an unexpected number to us,
> + * so let's do the check ourselves firstly.
> + */
> + if (str[0] == '-') {
> + fprintf(stderr, "ERROR: %s may be negative value.\n", str);
well, it _is_ a negative value right? (vs. "may be")
So perhaps:
fprintf(stderr, "ERROR: %s: negative value is invalid.\n", str);
> + exit(1);
> + }
> +
> + value = strtoull(str, &ptr_parse_end, 0);
> + if (ptr_parse_end != ptr_str_end) {
> + fprintf(stderr, "ERROR: %s is not valid value.\n", str);
maybe:
fprintf(stderr, "ERROR: %s is not a valid numeric value.\n", str);
Otherwise, this looks fine to me. We'll see what the others on the thread
think. :)
thanks,
-Eric
> + exit(1);
> + }
> + if (value == ULLONG_MAX) {
> + fprintf(stderr, "ERROR: %s is too large.\n", str);
> + exit(1);
> + }
> + return value;
> +}
> +
> u64 parse_size(char *s)
> {
> int i;
> diff --git a/utils.h b/utils.h
> index 04b8c45..a201085 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -71,6 +71,7 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_bytes);
> int get_mountpt(char *dev, char *mntpt, size_t size);
> int btrfs_scan_block_devices(int run_ioctl);
> u64 parse_size(char *s);
> +u64 arg_strtou64(const char *str);
> int open_file_or_dir(const char *fname, DIR **dirstream);
> void close_file_or_dir(int fd, DIR *dirstream);
> int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/4] Btrfs-progs: new helper to parse string to u64 for btrfs
2014-02-20 1:39 ` Eric Sandeen
@ 2014-02-20 1:43 ` Wang Shilong
2014-02-20 16:43 ` Eric Sandeen
0 siblings, 1 reply; 8+ messages in thread
From: Wang Shilong @ 2014-02-20 1:43 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-btrfs
On 02/20/2014 09:39 AM, Eric Sandeen wrote:
> On 2/19/14, 7:30 PM, Wang Shilong wrote:
>> There are many places that need parse string to u64 for btrfs commands,
>> in fact, we do such things *too casually*, using atoi/atol/atoll..is not
>> right at all, and even we don't check whether it is a valid string.
>>
>> Let's do everything more gracefully, we introduce a new helper
>> arg_strtou64() which will do all the necessary checks.If we fail to
>> parse string to u64, we will output message and exit directly, this is
>> something like what usage() is doing. It is ok to not return erro to
>> it's caller, because this function should be called when parsing arg
>> (just like usage!)
>>
>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>> ---
>> utils.c | 33 +++++++++++++++++++++++++++++++++
>> utils.h | 1 +
>> 2 files changed, 34 insertions(+)
>>
>> diff --git a/utils.c b/utils.c
>> index 97e23d5..d570967 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -1520,6 +1520,39 @@ scan_again:
>> return 0;
>> }
>>
>> +/*
>> + * This function should be only used when parsing
>> + * command arg, it won't return error to it's
>> + * caller and rather exit directly just like usage().
>> + */
>> +u64 arg_strtou64(const char *str)
>> +{
>> + u64 value;
>> + char *ptr_parse_end = NULL;
>> + char *ptr_str_end = (char *)str + strlen(str);
>> +
>> + /*
>> + * if we pass a negative number to strtoull,
>> + * it will return an unexpected number to us,
>> + * so let's do the check ourselves firstly.
>> + */
>> + if (str[0] == '-') {
>> + fprintf(stderr, "ERROR: %s may be negative value.\n", str);
> well, it _is_ a negative value right? (vs. "may be")
>
> So perhaps:
>
> fprintf(stderr, "ERROR: %s: negative value is invalid.\n", str);
I use "may be" because the following case:
-123xxxx, -abcd..... something like these, these string are invalid,
but they are not negative number...So i have not thought a better idea
to tell user what is wrong with input.:-)
>
>
>> + exit(1);
>> + }
>> +
>> + value = strtoull(str, &ptr_parse_end, 0);
>> + if (ptr_parse_end != ptr_str_end) {
>> + fprintf(stderr, "ERROR: %s is not valid value.\n", str);
> maybe:
>
> fprintf(stderr, "ERROR: %s is not a valid numeric value.\n", str);
Hm, better and better!
>
> Otherwise, this looks fine to me. We'll see what the others on the thread
> think. :)
>
> thanks,
> -Eric
>
>> + exit(1);
>> + }
>> + if (value == ULLONG_MAX) {
>> + fprintf(stderr, "ERROR: %s is too large.\n", str);
>> + exit(1);
>> + }
>> + return value;
>> +}
>> +
>> u64 parse_size(char *s)
>> {
>> int i;
>> diff --git a/utils.h b/utils.h
>> index 04b8c45..a201085 100644
>> --- a/utils.h
>> +++ b/utils.h
>> @@ -71,6 +71,7 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_bytes);
>> int get_mountpt(char *dev, char *mntpt, size_t size);
>> int btrfs_scan_block_devices(int run_ioctl);
>> u64 parse_size(char *s);
>> +u64 arg_strtou64(const char *str);
>> int open_file_or_dir(const char *fname, DIR **dirstream);
>> void close_file_or_dir(int fd, DIR *dirstream);
>> int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/4] Btrfs-progs: new helper to parse string to u64 for btrfs
2014-02-20 1:43 ` Wang Shilong
@ 2014-02-20 16:43 ` Eric Sandeen
0 siblings, 0 replies; 8+ messages in thread
From: Eric Sandeen @ 2014-02-20 16:43 UTC (permalink / raw)
To: Wang Shilong; +Cc: linux-btrfs
On 2/19/14, 7:43 PM, Wang Shilong wrote:
> On 02/20/2014 09:39 AM, Eric Sandeen wrote:
>> On 2/19/14, 7:30 PM, Wang Shilong wrote:
...
>>> + /*
>>> + * if we pass a negative number to strtoull,
>>> + * it will return an unexpected number to us,
>>> + * so let's do the check ourselves firstly.
>>> + */
>>> + if (str[0] == '-') {
>>> + fprintf(stderr, "ERROR: %s may be negative value.\n", str);
>> well, it _is_ a negative value right? (vs. "may be")
>>
>> So perhaps:
>>
>> fprintf(stderr, "ERROR: %s: negative value is invalid.\n", str);
> I use "may be" because the following case:
>
> -123xxxx, -abcd..... something like these, these string are invalid,
> but they are not negative number...So i have not thought a better idea
> to tell user what is wrong with input.:-)
Ok; well, sorry for being nitpicky. :) But user error messages probably
should be very clear and unambiguous; we may as well do this right.
So what about this:
Do strtoull first, and *if* it passes numeric parsing, but str[0] == '-',
*then* say "ERROR: %s: negative value is invalid."
-Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/4] Btrfs-progs: switch to arg_strtou64() part1
2014-02-20 1:30 [PATCH v2 0/4] Btrfs-progs: cleanups: new helper for parsing string to u64 Wang Shilong
2014-02-20 1:30 ` [PATCH v2 1/4] Btrfs-progs: new helper to parse string to u64 for btrfs Wang Shilong
@ 2014-02-20 1:30 ` Wang Shilong
2014-02-20 1:30 ` [PATCH v2 3/4] Btrfs-progs: switch to arg_strtou64() part2 Wang Shilong
2014-02-20 1:30 ` [PATCH v2 4/4] Btrfs-progs: switch to arg_strtou64() part3 Wang Shilong
3 siblings, 0 replies; 8+ messages in thread
From: Wang Shilong @ 2014-02-20 1:30 UTC (permalink / raw)
To: linux-btrfs
switch to arg_strtou64 plus some cleanups to remove unnecessary
codes.
Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
btrfs-find-root.c | 23 +++--------------------
btrfs-list.c | 14 +++-----------
cmds-restore.c | 27 +++++----------------------
utils.c | 2 +-
4 files changed, 12 insertions(+), 54 deletions(-)
diff --git a/btrfs-find-root.c b/btrfs-find-root.c
index 0ba4c57..db01923 100644
--- a/btrfs-find-root.c
+++ b/btrfs-find-root.c
@@ -289,30 +289,13 @@ int main(int argc, char **argv)
switch(opt) {
errno = 0;
case 'o':
- search_objectid = (u64)strtoll(optarg, NULL,
- 10);
- if (errno) {
- fprintf(stderr, "Error parsing "
- "objectid\n");
- exit(1);
- }
+ search_objectid = arg_strtou64(optarg);
break;
case 'g':
- search_generation = (u64)strtoll(optarg, NULL,
- 10);
- if (errno) {
- fprintf(stderr, "Error parsing "
- "generation\n");
- exit(1);
- }
+ search_generation = arg_strtou64(optarg);
break;
case 'l':
- search_level = strtol(optarg, NULL, 10);
- if (errno) {
- fprintf(stderr, "Error parsing "
- "level\n");
- exit(1);
- }
+ search_level = arg_strtou64(optarg);
break;
default:
usage();
diff --git a/btrfs-list.c b/btrfs-list.c
index 9effb27..912b27c 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -1854,32 +1854,24 @@ int btrfs_list_parse_filter_string(char *opt_arg,
{
u64 arg;
- char *ptr_parse_end = NULL;
- char *ptr_opt_arg_end = opt_arg + strlen(opt_arg);
switch (*(opt_arg++)) {
case '+':
- arg = (u64)strtol(opt_arg, &ptr_parse_end, 10);
+ arg = arg_strtou64(opt_arg);
type += 2;
- if (ptr_parse_end != ptr_opt_arg_end)
- return -1;
btrfs_list_setup_filter(filters, type, arg);
break;
case '-':
- arg = (u64)strtoll(opt_arg, &ptr_parse_end, 10);
+ arg = arg_strtou64(opt_arg);
type += 1;
- if (ptr_parse_end != ptr_opt_arg_end)
- return -1;
btrfs_list_setup_filter(filters, type, arg);
break;
default:
opt_arg--;
- arg = (u64)strtoll(opt_arg, &ptr_parse_end, 10);
+ arg = arg_strtou64(opt_arg);
- if (ptr_parse_end != ptr_opt_arg_end)
- return -1;
btrfs_list_setup_filter(filters, type, arg);
break;
}
diff --git a/cmds-restore.c b/cmds-restore.c
index fd533ce..6659c75 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -1160,26 +1160,14 @@ int cmd_restore(int argc, char **argv)
overwrite = 1;
break;
case 't':
- errno = 0;
- tree_location = (u64)strtoll(optarg, NULL, 10);
- if (errno != 0) {
- fprintf(stderr, "Tree location not valid\n");
- exit(1);
- }
+ tree_location = arg_strtou64(optarg);
break;
case 'f':
- errno = 0;
- fs_location = (u64)strtoll(optarg, NULL, 10);
- if (errno != 0) {
- fprintf(stderr, "Fs location not valid\n");
- exit(1);
- }
+ fs_location = arg_strtou64(optarg);
break;
case 'u':
- errno = 0;
- super_mirror = (int)strtol(optarg, NULL, 10);
- if (errno != 0 ||
- super_mirror >= BTRFS_SUPER_MIRROR_MAX) {
+ super_mirror = arg_strtou64(optarg);
+ if (super_mirror >= BTRFS_SUPER_MIRROR_MAX) {
fprintf(stderr, "Super mirror not "
"valid\n");
exit(1);
@@ -1189,12 +1177,7 @@ int cmd_restore(int argc, char **argv)
find_dir = 1;
break;
case 'r':
- errno = 0;
- root_objectid = (u64)strtoll(optarg, NULL, 10);
- if (errno != 0) {
- fprintf(stderr, "Root objectid not valid\n");
- exit(1);
- }
+ root_objectid = arg_strtou64(optarg);
break;
case 'l':
list_roots = 1;
diff --git a/utils.c b/utils.c
index d570967..c66c601 100644
--- a/utils.c
+++ b/utils.c
@@ -1600,7 +1600,7 @@ u64 parse_size(char *s)
s[i+1]);
exit(51);
}
- return strtoull(s, NULL, 10) * mult;
+ return arg_strtou64(s) * mult;
}
int open_file_or_dir(const char *fname, DIR **dirstream)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 3/4] Btrfs-progs: switch to arg_strtou64() part2
2014-02-20 1:30 [PATCH v2 0/4] Btrfs-progs: cleanups: new helper for parsing string to u64 Wang Shilong
2014-02-20 1:30 ` [PATCH v2 1/4] Btrfs-progs: new helper to parse string to u64 for btrfs Wang Shilong
2014-02-20 1:30 ` [PATCH v2 2/4] Btrfs-progs: switch to arg_strtou64() part1 Wang Shilong
@ 2014-02-20 1:30 ` Wang Shilong
2014-02-20 1:30 ` [PATCH v2 4/4] Btrfs-progs: switch to arg_strtou64() part3 Wang Shilong
3 siblings, 0 replies; 8+ messages in thread
From: Wang Shilong @ 2014-02-20 1:30 UTC (permalink / raw)
To: linux-btrfs
Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
btrfs-corrupt-block.c | 45 +++++++++------------------------------------
btrfs-debug-tree.c | 2 +-
btrfs-image.c | 12 ++++++------
btrfs-map-logical.c | 26 ++++++--------------------
cmds-inspect.c | 8 ++++----
5 files changed, 26 insertions(+), 67 deletions(-)
diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
index 10cae00..6ecbe47 100644
--- a/btrfs-corrupt-block.c
+++ b/btrfs-corrupt-block.c
@@ -36,7 +36,7 @@
#define FIELD_BUF_LEN 80
struct extent_buffer *debug_corrupt_block(struct btrfs_root *root, u64 bytenr,
- u32 blocksize, int copy)
+ u32 blocksize, u64 copy)
{
int ret;
struct extent_buffer *eb;
@@ -165,7 +165,7 @@ static int corrupt_keys_in_block(struct btrfs_root *root, u64 bytenr)
}
static int corrupt_extent(struct btrfs_trans_handle *trans,
- struct btrfs_root *root, u64 bytenr, int copy)
+ struct btrfs_root *root, u64 bytenr, u64 copy)
{
struct btrfs_key key;
struct extent_buffer *leaf;
@@ -792,7 +792,7 @@ int main(int ac, char **av)
u64 logical = (u64)-1;
int ret = 0;
int option_index = 0;
- int copy = 0;
+ u64 copy = 0;
u64 bytes = 4096;
int extent_rec = 0;
int extent_tree = 0;
@@ -816,23 +816,13 @@ int main(int ac, char **av)
break;
switch(c) {
case 'l':
- logical = atoll(optarg);
+ logical = arg_strtou64(optarg);
break;
case 'c':
- copy = atoi(optarg);
- if (copy <= 0) {
- fprintf(stderr,
- "invalid copy number\n");
- print_usage();
- }
+ copy = arg_strtou64(optarg);
break;
case 'b':
- bytes = atoll(optarg);
- if (bytes == 0) {
- fprintf(stderr,
- "invalid byte count\n");
- print_usage();
- }
+ bytes = arg_strtou64(optarg);
break;
case 'e':
extent_rec = 1;
@@ -849,33 +839,16 @@ int main(int ac, char **av)
case 'U':
chunk_tree = 1;
case 'i':
- inode = atoll(optarg);
- if (inode == 0) {
- fprintf(stderr,
- "invalid inode number\n");
- print_usage();
- }
+ inode = arg_strtou64(optarg);
break;
case 'f':
strncpy(field, optarg, FIELD_BUF_LEN);
break;
case 'x':
- errno = 0;
- file_extent = atoll(optarg);
- if (errno) {
- fprintf(stderr, "error converting "
- "%d\n", errno);
- print_usage();
- }
+ file_extent = arg_strtou64(optarg);
break;
case 'm':
- errno = 0;
- metadata_block = atoll(optarg);
- if (errno) {
- fprintf(stderr, "error converting "
- "%d\n", errno);
- print_usage();
- }
+ metadata_block = arg_strtou64(optarg);
break;
case 'K':
ret = sscanf(optarg, "%llu,%u,%llu",
diff --git a/btrfs-debug-tree.c b/btrfs-debug-tree.c
index f37de9d..8ae7270 100644
--- a/btrfs-debug-tree.c
+++ b/btrfs-debug-tree.c
@@ -162,7 +162,7 @@ int main(int ac, char **av)
root_backups = 1;
break;
case 'b':
- block_only = atoll(optarg);
+ block_only = arg_strtou64(optarg);
break;
default:
print_usage();
diff --git a/btrfs-image.c b/btrfs-image.c
index 1b2831a..1fc641f 100644
--- a/btrfs-image.c
+++ b/btrfs-image.c
@@ -2463,8 +2463,8 @@ int main(int argc, char *argv[])
{
char *source;
char *target;
- int num_threads = 0;
- int compress_level = 0;
+ u64 num_threads = 0;
+ u64 compress_level = 0;
int create = 1;
int old_restore = 0;
int walk_trees = 0;
@@ -2483,13 +2483,13 @@ int main(int argc, char *argv[])
create = 0;
break;
case 't':
- num_threads = atoi(optarg);
- if (num_threads <= 0 || num_threads > 32)
+ num_threads = arg_strtou64(optarg);
+ if (num_threads > 32)
print_usage();
break;
case 'c':
- compress_level = atoi(optarg);
- if (compress_level < 0 || compress_level > 9)
+ compress_level = arg_strtou64(optarg);
+ if (compress_level > 9)
print_usage();
break;
case 'o':
diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c
index 51179a3..a907e4d 100644
--- a/btrfs-map-logical.c
+++ b/btrfs-map-logical.c
@@ -31,6 +31,7 @@
#include "transaction.h"
#include "list.h"
#include "version.h"
+#include "utils.h"
/* we write the mirror info to stdout unless they are dumping the data
* to stdout
@@ -38,7 +39,7 @@
static FILE *info_file;
static struct extent_buffer * debug_read_block(struct btrfs_root *root,
- u64 bytenr, u32 blocksize, int copy)
+ u64 bytenr, u32 blocksize, u64 copy)
{
int ret;
struct extent_buffer *eb;
@@ -120,7 +121,7 @@ int main(int ac, char **av)
u64 logical = 0;
int ret = 0;
int option_index = 0;
- int copy = 0;
+ u64 copy = 0;
u64 bytes = 0;
int out_fd = 0;
@@ -132,28 +133,13 @@ int main(int ac, char **av)
break;
switch(c) {
case 'l':
- logical = atoll(optarg);
- if (logical == 0) {
- fprintf(stderr,
- "invalid extent number\n");
- print_usage();
- }
+ logical = arg_strtou64(optarg);
break;
case 'c':
- copy = atoi(optarg);
- if (copy == 0) {
- fprintf(stderr,
- "invalid copy number\n");
- print_usage();
- }
+ copy = arg_strtou64(optarg);
break;
case 'b':
- bytes = atoll(optarg);
- if (bytes == 0) {
- fprintf(stderr,
- "invalid byte count\n");
- print_usage();
- }
+ bytes = arg_strtou64(optarg);
break;
case 'o':
output_file = strdup(optarg);
diff --git a/cmds-inspect.c b/cmds-inspect.c
index f0c8e3d..cd9d2c6 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -120,7 +120,7 @@ static int cmd_inode_resolve(int argc, char **argv)
return 1;
}
- ret = __ino_to_path_fd(atoll(argv[optind]), fd, verbose,
+ ret = __ino_to_path_fd(arg_strtou64(argv[optind]), fd, verbose,
argv[optind+1]);
close_file_or_dir(fd, dirstream);
return !!ret;
@@ -167,7 +167,7 @@ static int cmd_logical_resolve(int argc, char **argv)
verbose = 1;
break;
case 's':
- size = atoll(optarg);
+ size = arg_strtou64(optarg);
break;
default:
usage(cmd_logical_resolve_usage);
@@ -183,7 +183,7 @@ static int cmd_logical_resolve(int argc, char **argv)
return 1;
memset(inodes, 0, sizeof(*inodes));
- loi.logical = atoll(argv[optind]);
+ loi.logical = arg_strtou64(argv[optind]);
loi.size = size;
loi.inodes = (uintptr_t)inodes;
@@ -283,7 +283,7 @@ static int cmd_subvolid_resolve(int argc, char **argv)
goto out;
}
- subvol_id = atoll(argv[1]);
+ subvol_id = arg_strtou64(argv[1]);
ret = btrfs_subvolid_resolve(fd, path, sizeof(path), subvol_id);
if (ret) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 4/4] Btrfs-progs: switch to arg_strtou64() part3
2014-02-20 1:30 [PATCH v2 0/4] Btrfs-progs: cleanups: new helper for parsing string to u64 Wang Shilong
` (2 preceding siblings ...)
2014-02-20 1:30 ` [PATCH v2 3/4] Btrfs-progs: switch to arg_strtou64() part2 Wang Shilong
@ 2014-02-20 1:30 ` Wang Shilong
3 siblings, 0 replies; 8+ messages in thread
From: Wang Shilong @ 2014-02-20 1:30 UTC (permalink / raw)
To: linux-btrfs
Switch to new helper arg_strtou64(), also check if user assign
a valid super copy.
Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
btrfs-select-super.c | 12 +++++++++---
btrfs-show-super.c | 10 +++++-----
btrfstune.c | 4 ++--
cmds-check.c | 14 ++++++++++----
cmds-replace.c | 7 +------
cmds-subvolume.c | 8 ++------
6 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/btrfs-select-super.c b/btrfs-select-super.c
index 6a458b8..15e6921 100644
--- a/btrfs-select-super.c
+++ b/btrfs-select-super.c
@@ -43,7 +43,7 @@ int main(int ac, char **av)
{
struct btrfs_root *root;
int ret;
- int num = 0;
+ u64 num = 0;
u64 bytenr = 0;
while(1) {
@@ -53,8 +53,14 @@ int main(int ac, char **av)
break;
switch(c) {
case 's':
- num = atol(optarg);
- bytenr = btrfs_sb_offset(num);
+ num = arg_strtou64(optarg);
+ if (num >= BTRFS_SUPER_MIRROR_MAX) {
+ fprintf(stderr,
+ "ERROR: super mirror should be less than: %d\n",
+ BTRFS_SUPER_MIRROR_MAX);
+ exit(1);
+ }
+ bytenr = btrfs_sb_offset(((int)num));
break;
default:
print_usage();
diff --git a/btrfs-show-super.c b/btrfs-show-super.c
index b87f16a..d4df0ac 100644
--- a/btrfs-show-super.c
+++ b/btrfs-show-super.c
@@ -59,17 +59,17 @@ int main(int argc, char **argv)
int all = 0;
char *filename;
int fd = -1;
- int arg, i;
+ int i;
+ u64 arg;
u64 sb_bytenr = btrfs_sb_offset(0);
while ((opt = getopt(argc, argv, "ai:")) != -1) {
switch (opt) {
case 'i':
- arg = atoi(optarg);
-
- if (arg < 0 || arg >= BTRFS_SUPER_MIRROR_MAX) {
+ arg = arg_strtou64(optarg);
+ if (arg >= BTRFS_SUPER_MIRROR_MAX) {
fprintf(stderr,
- "Illegal super_mirror %d\n",
+ "Illegal super_mirror %llu\n",
arg);
print_usage();
exit(1);
diff --git a/btrfstune.c b/btrfstune.c
index da82f36..855427f 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -111,7 +111,7 @@ int main(int argc, char *argv[])
int success = 0;
int extrefs_flag = 0;
int seeding_flag = 0;
- int seeding_value = 0;
+ u64 seeding_value = 0;
int skinny_flag = 0;
int ret;
@@ -123,7 +123,7 @@ int main(int argc, char *argv[])
switch(c) {
case 'S':
seeding_flag = 1;
- seeding_value = atoi(optarg);
+ seeding_value = arg_strtou64(optarg);
break;
case 'r':
extrefs_flag = 1;
diff --git a/cmds-check.c b/cmds-check.c
index 61c1815..a2afae6 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -6388,7 +6388,7 @@ int cmd_check(int argc, char **argv)
u64 bytenr = 0;
char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
int ret;
- int num;
+ u64 num;
int option_index = 0;
int init_csum_tree = 0;
int init_extent_tree = 0;
@@ -6407,9 +6407,15 @@ int cmd_check(int argc, char **argv)
ctree_flags |= OPEN_CTREE_BACKUP_ROOT;
break;
case 's':
- num = atol(optarg);
- bytenr = btrfs_sb_offset(num);
- printf("using SB copy %d, bytenr %llu\n", num,
+ num = arg_strtou64(optarg);
+ if (num >= BTRFS_SUPER_MIRROR_MAX) {
+ fprintf(stderr,
+ "ERROR: super mirror should be less than: %d\n",
+ BTRFS_SUPER_MIRROR_MAX);
+ exit(1);
+ }
+ bytenr = btrfs_sb_offset(((int)num));
+ printf("using SB copy %llu, bytenr %llu\n", num,
(unsigned long long)bytenr);
break;
case '?':
diff --git a/cmds-replace.c b/cmds-replace.c
index c683d6c..01ab77c 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -210,12 +210,7 @@ static int cmd_start_replace(int argc, char **argv)
struct btrfs_ioctl_fs_info_args fi_args;
struct btrfs_ioctl_dev_info_args *di_args = NULL;
- if (atoi(srcdev) == 0) {
- fprintf(stderr, "Error: Failed to parse the numerical devid value '%s'\n",
- srcdev);
- goto leave_with_error;
- }
- start_args.start.srcdevid = (__u64)atoi(srcdev);
+ start_args.start.srcdevid = arg_strtou64(srcdev);
ret = get_fs_info(path, &fi_args, &di_args);
if (ret) {
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 0bd76f2..5e821c7 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -820,11 +820,7 @@ static int cmd_subvol_set_default(int argc, char **argv)
subvolid = argv[1];
path = argv[2];
- objectid = (unsigned long long)strtoll(subvolid, NULL, 0);
- if (errno == ERANGE) {
- fprintf(stderr, "ERROR: invalid tree id (%s)\n", subvolid);
- return 1;
- }
+ objectid = arg_strtou64(subvolid);
fd = open_file_or_dir(path, &dirstream);
if (fd < 0) {
@@ -861,7 +857,7 @@ static int cmd_find_new(int argc, char **argv)
usage(cmd_find_new_usage);
subvol = argv[1];
- last_gen = atoll(argv[2]);
+ last_gen = arg_strtou64(argv[2]);
ret = test_issubvolume(subvol);
if (ret < 0) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread