Re: [PATCH 18/20] Btrfs-progs: fix magic return value in cmds-balance.c

2013-09-05 Thread Wang Shilong

Hi Illya,

On 09/05/2013 12:26 AM, Ilya Dryomov wrote:

Hi Wang,

Thank you for doing the grunt work, it's been a long standing todo.
See the comments below.

On Wed, Sep 4, 2013 at 6:22 PM, Wang Shilong wangshilong1...@gmail.com wrote:

From: Wang Shilong wangsl.f...@cn.fujitsu.com

If there is no balance in progress.resume/pause/cancel will
return 2. For usage or syntal errors will return 1. 0 means
operations return successfully.

This needs to be reworded (spelling, punctuation).

will fix this.



Signed-off-by: Wang Shilong wangsl.f...@cn.fujitsu.com
---
  cmds-balance.c | 93 --
  1 file changed, 58 insertions(+), 35 deletions(-)

diff --git a/cmds-balance.c b/cmds-balance.c
index b7382ef..fd68051 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -58,7 +58,7 @@ static int parse_one_profile(const char *profile, u64 *flags)
 *flags |= BTRFS_AVAIL_ALLOC_BIT_SINGLE;
 } else {
 fprintf(stderr, Unknown profile '%s'\n, profile);
-   return 1;
+   return -ENOENT;
 }

 return 0;
@@ -68,12 +68,14 @@ static int parse_profiles(char *profiles, u64 *flags)
  {
 char *this_char;
 char *save_ptr = NULL; /* Satisfy static checkers */
+   int ret;

 for (this_char = strtok_r(profiles, |, save_ptr);
  this_char != NULL;
  this_char = strtok_r(NULL, |, save_ptr)) {
-   if (parse_one_profile(this_char, flags))
-   return 1;
+   ret = parse_one_profile(this_char, flags);
+   if (ret)
+   return ret;
 }

 return 0;
@@ -86,7 +88,7 @@ static int parse_u64(const char *str, u64 *result)

 val = strtoull(str, endptr, 10);
 if (*endptr)
-   return 1;
+   return -EINVAL;

 *result = val;
 return 0;
@@ -95,6 +97,7 @@ static int parse_u64(const char *str, u64 *result)
  static int parse_range(const char *range, u64 *start, u64 *end)
  {
 char *dots;
+   int ret;

 dots = strstr(range, ..);
 if (dots) {
@@ -107,29 +110,31 @@ static int parse_range(const char *range, u64 *start, u64 
*end)
 *end = (u64)-1;
 skipped++;
 } else {
-   if (parse_u64(rest, end))
-   return 1;
+   ret = parse_u64(rest, end);
+   if (ret)
+   return ret;
 }
 if (dots == range) {
 *start = 0;
 skipped++;
 } else {
+   ret = parse_u64(rest, end);
 if (parse_u64(range, start))
-   return 1;
+   return ret;
 }

 if (*start = *end) {
 fprintf(stderr, Range %llu..%llu doesn't make 
 sense\n, (unsigned long long)*start,
 (unsigned long long)*end);
-   return 1;
+   return -EINVAL;
 }

 if (skipped = 1)
 return 0;
 }

-   return 1;
+   return -EINVAL;
  }

  static int parse_filters(char *filters, struct btrfs_balance_args *args)
@@ -137,6 +142,7 @@ static int parse_filters(char *filters, struct 
btrfs_balance_args *args)
 char *this_char;
 char *value;
 char *save_ptr = NULL; /* Satisfy static checkers */
+   int ret = 0;

 if (!filters)
 return 0;
@@ -150,70 +156,72 @@ static int parse_filters(char *filters, struct 
btrfs_balance_args *args)
 if (!value || !*value) {
 fprintf(stderr, the profiles filter requires 
an argument\n);
-   return 1;
+   return -EINVAL;
 }
 if (parse_profiles(value, args-profiles)) {
 fprintf(stderr, Invalid profiles argument\n);
-   return 1;
+   return -EINVAL;
 }
 args-flags |= BTRFS_BALANCE_ARGS_PROFILES;
 } else if (!strcmp(this_char, usage)) {
 if (!value || !*value) {
 fprintf(stderr, the usage filter requires 
an argument\n);
-   return 1;
+   return -EINVAL;
 }
 if (parse_u64(value, args-usage) ||
 args-usage  100) {
 

Re: [PATCH 18/20] Btrfs-progs: fix magic return value in cmds-balance.c

2013-09-05 Thread Stefan Behrens
On Thu, 05 Sep 2013 15:44:11 +0800, Wang Shilong wrote:
[..]
 @@ -297,9 +305,10 @@ static int do_balance(const char *path, struct
 btrfs_ioctl_balance_args *args,
  DIR *dirstream = NULL;

  fd = open_file_or_dir(path, dirstream);
 +   e = errno;
  if (fd  0) {
  fprintf(stderr, ERROR: can't access to '%s'\n, path);
 -   return 12;
 +   return e;

Since I didn't understand whether you rejected or acknowledged Ilya's
comments, if you don't do so, please change the above line to return
-e like it is everywhere else: errno is returned as a negative value, 0
means no error, a positive value means the function failed but the
return value cannot be interpreted as an errno.
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 18/20] Btrfs-progs: fix magic return value in cmds-balance.c

2013-09-05 Thread Wang Shilong

On 09/05/2013 04:21 PM, Stefan Behrens wrote:

On Thu, 05 Sep 2013 15:44:11 +0800, Wang Shilong wrote:
[..]

@@ -297,9 +305,10 @@ static int do_balance(const char *path, struct
btrfs_ioctl_balance_args *args,
  DIR *dirstream = NULL;

  fd = open_file_or_dir(path, dirstream);
+   e = errno;
  if (fd  0) {
  fprintf(stderr, ERROR: can't access to '%s'\n, path);
-   return 12;
+   return e;

Since I didn't understand whether you rejected or acknowledged Ilya's

My answer: acknowledged.

Will send a V2 later, just wait more comments.

Thanks,
Wang

comments, if you don't do so, please change the above line to return
-e like it is everywhere else: errno is returned as a negative value, 0
means no error, a positive value means the function failed but the
return value cannot be interpreted as an errno.
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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


[PATCH 18/20] Btrfs-progs: fix magic return value in cmds-balance.c

2013-09-04 Thread Wang Shilong
From: Wang Shilong wangsl.f...@cn.fujitsu.com

If there is no balance in progress.resume/pause/cancel will
return 2. For usage or syntal errors will return 1. 0 means
operations return successfully.

Signed-off-by: Wang Shilong wangsl.f...@cn.fujitsu.com
---
 cmds-balance.c | 93 --
 1 file changed, 58 insertions(+), 35 deletions(-)

diff --git a/cmds-balance.c b/cmds-balance.c
index b7382ef..fd68051 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -58,7 +58,7 @@ static int parse_one_profile(const char *profile, u64 *flags)
*flags |= BTRFS_AVAIL_ALLOC_BIT_SINGLE;
} else {
fprintf(stderr, Unknown profile '%s'\n, profile);
-   return 1;
+   return -ENOENT;
}
 
return 0;
@@ -68,12 +68,14 @@ static int parse_profiles(char *profiles, u64 *flags)
 {
char *this_char;
char *save_ptr = NULL; /* Satisfy static checkers */
+   int ret;
 
for (this_char = strtok_r(profiles, |, save_ptr);
 this_char != NULL;
 this_char = strtok_r(NULL, |, save_ptr)) {
-   if (parse_one_profile(this_char, flags))
-   return 1;
+   ret = parse_one_profile(this_char, flags);
+   if (ret)
+   return ret;
}
 
return 0;
@@ -86,7 +88,7 @@ static int parse_u64(const char *str, u64 *result)
 
val = strtoull(str, endptr, 10);
if (*endptr)
-   return 1;
+   return -EINVAL;
 
*result = val;
return 0;
@@ -95,6 +97,7 @@ static int parse_u64(const char *str, u64 *result)
 static int parse_range(const char *range, u64 *start, u64 *end)
 {
char *dots;
+   int ret;
 
dots = strstr(range, ..);
if (dots) {
@@ -107,29 +110,31 @@ static int parse_range(const char *range, u64 *start, u64 
*end)
*end = (u64)-1;
skipped++;
} else {
-   if (parse_u64(rest, end))
-   return 1;
+   ret = parse_u64(rest, end);
+   if (ret)
+   return ret;
}
if (dots == range) {
*start = 0;
skipped++;
} else {
+   ret = parse_u64(rest, end);
if (parse_u64(range, start))
-   return 1;
+   return ret;
}
 
if (*start = *end) {
fprintf(stderr, Range %llu..%llu doesn't make 
sense\n, (unsigned long long)*start,
(unsigned long long)*end);
-   return 1;
+   return -EINVAL;
}
 
if (skipped = 1)
return 0;
}
 
-   return 1;
+   return -EINVAL;
 }
 
 static int parse_filters(char *filters, struct btrfs_balance_args *args)
@@ -137,6 +142,7 @@ static int parse_filters(char *filters, struct 
btrfs_balance_args *args)
char *this_char;
char *value;
char *save_ptr = NULL; /* Satisfy static checkers */
+   int ret = 0;
 
if (!filters)
return 0;
@@ -150,70 +156,72 @@ static int parse_filters(char *filters, struct 
btrfs_balance_args *args)
if (!value || !*value) {
fprintf(stderr, the profiles filter requires 
   an argument\n);
-   return 1;
+   return -EINVAL;
}
if (parse_profiles(value, args-profiles)) {
fprintf(stderr, Invalid profiles argument\n);
-   return 1;
+   return -EINVAL;
}
args-flags |= BTRFS_BALANCE_ARGS_PROFILES;
} else if (!strcmp(this_char, usage)) {
if (!value || !*value) {
fprintf(stderr, the usage filter requires 
   an argument\n);
-   return 1;
+   return -EINVAL;
}
if (parse_u64(value, args-usage) ||
args-usage  100) {
fprintf(stderr, Invalid usage argument: %s\n,
   value);
-   return 1;
+   return -EINVAL;
}
args-flags |= BTRFS_BALANCE_ARGS_USAGE;
} else if (!strcmp(this_char, devid)) {
if (!value 

Re: [PATCH 18/20] Btrfs-progs: fix magic return value in cmds-balance.c

2013-09-04 Thread Ilya Dryomov
Hi Wang,

Thank you for doing the grunt work, it's been a long standing todo.
See the comments below.

On Wed, Sep 4, 2013 at 6:22 PM, Wang Shilong wangshilong1...@gmail.com wrote:
 From: Wang Shilong wangsl.f...@cn.fujitsu.com

 If there is no balance in progress.resume/pause/cancel will
 return 2. For usage or syntal errors will return 1. 0 means
 operations return successfully.

This needs to be reworded (spelling, punctuation).


 Signed-off-by: Wang Shilong wangsl.f...@cn.fujitsu.com
 ---
  cmds-balance.c | 93 
 --
  1 file changed, 58 insertions(+), 35 deletions(-)

 diff --git a/cmds-balance.c b/cmds-balance.c
 index b7382ef..fd68051 100644
 --- a/cmds-balance.c
 +++ b/cmds-balance.c
 @@ -58,7 +58,7 @@ static int parse_one_profile(const char *profile, u64 
 *flags)
 *flags |= BTRFS_AVAIL_ALLOC_BIT_SINGLE;
 } else {
 fprintf(stderr, Unknown profile '%s'\n, profile);
 -   return 1;
 +   return -ENOENT;
 }

 return 0;
 @@ -68,12 +68,14 @@ static int parse_profiles(char *profiles, u64 *flags)
  {
 char *this_char;
 char *save_ptr = NULL; /* Satisfy static checkers */
 +   int ret;

 for (this_char = strtok_r(profiles, |, save_ptr);
  this_char != NULL;
  this_char = strtok_r(NULL, |, save_ptr)) {
 -   if (parse_one_profile(this_char, flags))
 -   return 1;
 +   ret = parse_one_profile(this_char, flags);
 +   if (ret)
 +   return ret;
 }

 return 0;
 @@ -86,7 +88,7 @@ static int parse_u64(const char *str, u64 *result)

 val = strtoull(str, endptr, 10);
 if (*endptr)
 -   return 1;
 +   return -EINVAL;

 *result = val;
 return 0;
 @@ -95,6 +97,7 @@ static int parse_u64(const char *str, u64 *result)
  static int parse_range(const char *range, u64 *start, u64 *end)
  {
 char *dots;
 +   int ret;

 dots = strstr(range, ..);
 if (dots) {
 @@ -107,29 +110,31 @@ static int parse_range(const char *range, u64 *start, 
 u64 *end)
 *end = (u64)-1;
 skipped++;
 } else {
 -   if (parse_u64(rest, end))
 -   return 1;
 +   ret = parse_u64(rest, end);
 +   if (ret)
 +   return ret;
 }
 if (dots == range) {
 *start = 0;
 skipped++;
 } else {
 +   ret = parse_u64(rest, end);
 if (parse_u64(range, start))
 -   return 1;
 +   return ret;
 }

 if (*start = *end) {
 fprintf(stderr, Range %llu..%llu doesn't make 
 sense\n, (unsigned long long)*start,
 (unsigned long long)*end);
 -   return 1;
 +   return -EINVAL;
 }

 if (skipped = 1)
 return 0;
 }

 -   return 1;
 +   return -EINVAL;
  }

  static int parse_filters(char *filters, struct btrfs_balance_args *args)
 @@ -137,6 +142,7 @@ static int parse_filters(char *filters, struct 
 btrfs_balance_args *args)
 char *this_char;
 char *value;
 char *save_ptr = NULL; /* Satisfy static checkers */
 +   int ret = 0;

 if (!filters)
 return 0;
 @@ -150,70 +156,72 @@ static int parse_filters(char *filters, struct 
 btrfs_balance_args *args)
 if (!value || !*value) {
 fprintf(stderr, the profiles filter requires 
 
an argument\n);
 -   return 1;
 +   return -EINVAL;
 }
 if (parse_profiles(value, args-profiles)) {
 fprintf(stderr, Invalid profiles 
 argument\n);
 -   return 1;
 +   return -EINVAL;
 }
 args-flags |= BTRFS_BALANCE_ARGS_PROFILES;
 } else if (!strcmp(this_char, usage)) {
 if (!value || !*value) {
 fprintf(stderr, the usage filter requires 
an argument\n);
 -   return 1;
 +   return -EINVAL;
 }
 if (parse_u64(value, args-usage) ||
 args-usage  100) {
 fprintf(stderr,