* [PATCH] xfsprogs: merge the duplicated condition in xfs_mkfs.c/validate_ag_geometry
@ 2015-05-28 7:06 Wang Sheng-Hui
2015-05-28 14:55 ` Eric Sandeen
0 siblings, 1 reply; 3+ messages in thread
From: Wang Sheng-Hui @ 2015-05-28 7:06 UTC (permalink / raw)
To: xfs, dchinner, david, bfoster
Some conditions are duplicated. Merge them.
Signed-off-by: Wang Sheng-Hui <shhuiw@foxmail.com>
---
mkfs/xfs_mkfs.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index e2a052d..7ec2556 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -740,22 +740,6 @@ validate_ag_geometry(
__uint64_t agsize,
__uint64_t agcount)
{
- if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
- fprintf(stderr,
- _("agsize (%lld blocks) too small, need at least %lld blocks\n"),
- (long long)agsize,
- (long long)XFS_AG_MIN_BLOCKS(blocklog));
- usage();
- }
-
- if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
- fprintf(stderr,
- _("agsize (%lld blocks) too big, maximum is %lld blocks\n"),
- (long long)agsize,
- (long long)XFS_AG_MAX_BLOCKS(blocklog));
- usage();
- }
-
if (agsize > dblocks) {
fprintf(stderr,
_("agsize (%lld blocks) too big, data area is %lld blocks\n"),
@@ -765,6 +749,10 @@ validate_ag_geometry(
if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
fprintf(stderr,
+ _("agsize (%lld blocks) too small, need at least %lld blocks\n"),
+ (long long)agsize,
+ (long long)XFS_AG_MIN_BLOCKS(blocklog));
+ fprintf(stderr,
_("too many allocation groups for size = %lld\n"),
(long long)agsize);
fprintf(stderr, _("need at most %lld allocation groups\n"),
@@ -775,6 +763,10 @@ validate_ag_geometry(
if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
fprintf(stderr,
+ _("agsize (%lld blocks) too big, maximum is %lld blocks\n"),
+ (long long)agsize,
+ (long long)XFS_AG_MAX_BLOCKS(blocklog));
+ fprintf(stderr,
_("too few allocation groups for size = %lld\n"), (long long)agsize);
fprintf(stderr,
_("need at least %lld allocation groups\n"),
--
1.8.3.2
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] xfsprogs: merge the duplicated condition in xfs_mkfs.c/validate_ag_geometry
2015-05-28 7:06 [PATCH] xfsprogs: merge the duplicated condition in xfs_mkfs.c/validate_ag_geometry Wang Sheng-Hui
@ 2015-05-28 14:55 ` Eric Sandeen
2015-05-29 0:05 ` Wang Sheng-Hui
0 siblings, 1 reply; 3+ messages in thread
From: Eric Sandeen @ 2015-05-28 14:55 UTC (permalink / raw)
To: Wang Sheng-Hui, xfs, dchinner, david, bfoster
On 5/28/15 2:06 AM, Wang Sheng-Hui wrote:
> Some conditions are duplicated. Merge them.
While you are correct that they are duplicated, simply combining
them doesn't mkae a lot of sense; now we'll get this sort of
error message:
> agsize (%lld blocks) too small, need at least %lld blocks
> too many allocation groups for size = %lld
> need at most %lld allocation groups
and what should a user do with that information? It's somewhat
nonsensical.
The code as it is today, and as it has been since commit
1f1b8be7926480046ead7b98c9850ace7bcd82a3, doesn't make much sense,
because we can never hit the second conditional. But simply
combining them doesn't look like the right answer to me.
If you look at the above commit, what it used to do was something
like:
- /*
- * If the specified agsize is too small, or too large,
- * complain.
- */
- if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
- fprintf(stderr,
- _("agsize (%lldb) too small, need at least %lld blocks\n"),
- (long long)agsize,
- (long long)XFS_AG_MIN_BLOCKS(blocklog));
- usage();
- }
<snip>
- /*
- * If the ag size is too small, complain if agcount/agsize was
- * specified, and fix it otherwise.
- */
- if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
- if (daflag || dasize) {
- fprintf(stderr,
- _("too many allocation groups for size = %lld\n"),
- (long long)agsize);
- fprintf(stderr,
- _("need at most %lld allocation groups\n"),
- (long long)
- (dblocks / XFS_AG_MIN_BLOCKS(blocklog) +
- (dblocks % XFS_AG_MIN_BLOCKS(blocklog) != 0)));
...
I *think* the intent was to use different messages based on what
was specified by the user; i.e. if agsize was specified, and is too
small, then say:
> agsize (%lld blocks) too small, need at least %lld blocks
but if agcount was specified, and was too large, then say:
> too many allocation groups for size = %lld
> need at most %lld allocation groups
but combining those two messages doesn't make sense to me.
-Eric
> Signed-off-by: Wang Sheng-Hui <shhuiw@foxmail.com>
> ---
> mkfs/xfs_mkfs.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index e2a052d..7ec2556 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -740,22 +740,6 @@ validate_ag_geometry(
> __uint64_t agsize,
> __uint64_t agcount)
> {
> - if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
> - fprintf(stderr,
> - _("agsize (%lld blocks) too small, need at least %lld blocks\n"),
> - (long long)agsize,
> - (long long)XFS_AG_MIN_BLOCKS(blocklog));
> - usage();
> - }
> -
> - if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
> - fprintf(stderr,
> - _("agsize (%lld blocks) too big, maximum is %lld blocks\n"),
> - (long long)agsize,
> - (long long)XFS_AG_MAX_BLOCKS(blocklog));
> - usage();
> - }
> -
> if (agsize > dblocks) {
> fprintf(stderr,
> _("agsize (%lld blocks) too big, data area is %lld blocks\n"),
> @@ -765,6 +749,10 @@ validate_ag_geometry(
>
> if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
> fprintf(stderr,
> + _("agsize (%lld blocks) too small, need at least %lld blocks\n"),
> + (long long)agsize,
> + (long long)XFS_AG_MIN_BLOCKS(blocklog));
> + fprintf(stderr,
> _("too many allocation groups for size = %lld\n"),
> (long long)agsize);
> fprintf(stderr, _("need at most %lld allocation groups\n"),
> @@ -775,6 +763,10 @@ validate_ag_geometry(
>
> if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
> fprintf(stderr,
> + _("agsize (%lld blocks) too big, maximum is %lld blocks\n"),
> + (long long)agsize,
> + (long long)XFS_AG_MAX_BLOCKS(blocklog));
> + fprintf(stderr,
> _("too few allocation groups for size = %lld\n"), (long long)agsize);
> fprintf(stderr,
> _("need at least %lld allocation groups\n"),
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfsprogs: merge the duplicated condition in xfs_mkfs.c/validate_ag_geometry
2015-05-28 14:55 ` Eric Sandeen
@ 2015-05-29 0:05 ` Wang Sheng-Hui
0 siblings, 0 replies; 3+ messages in thread
From: Wang Sheng-Hui @ 2015-05-29 0:05 UTC (permalink / raw)
To: Eric Sandeen, xfs, dchinner, david, bfoster
On 2015年05月28日 22:55, Eric Sandeen wrote:
> On 5/28/15 2:06 AM, Wang Sheng-Hui wrote:
>> Some conditions are duplicated. Merge them.
>
> While you are correct that they are duplicated, simply combining
> them doesn't mkae a lot of sense; now we'll get this sort of
> error message:
>
>> agsize (%lld blocks) too small, need at least %lld blocks
>> too many allocation groups for size = %lld
>> need at most %lld allocation groups
>
> and what should a user do with that information? It's somewhat
> nonsensical.
>
> The code as it is today, and as it has been since commit
> 1f1b8be7926480046ead7b98c9850ace7bcd82a3, doesn't make much sense,
> because we can never hit the second conditional. But simply
> combining them doesn't look like the right answer to me.
>
> If you look at the above commit, what it used to do was something
> like:
>
> - /*
> - * If the specified agsize is too small, or too large,
> - * complain.
> - */
> - if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
> - fprintf(stderr,
> - _("agsize (%lldb) too small, need at least %lld blocks\n"),
> - (long long)agsize,
> - (long long)XFS_AG_MIN_BLOCKS(blocklog));
> - usage();
> - }
>
> <snip>
>
> - /*
> - * If the ag size is too small, complain if agcount/agsize was
> - * specified, and fix it otherwise.
> - */
> - if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
> - if (daflag || dasize) {
> - fprintf(stderr,
> - _("too many allocation groups for size = %lld\n"),
> - (long long)agsize);
> - fprintf(stderr,
> - _("need at most %lld allocation groups\n"),
> - (long long)
> - (dblocks / XFS_AG_MIN_BLOCKS(blocklog) +
> - (dblocks % XFS_AG_MIN_BLOCKS(blocklog) != 0)));
>
> ...
>
> I *think* the intent was to use different messages based on what
> was specified by the user; i.e. if agsize was specified, and is too
> small, then say:
>
>> agsize (%lld blocks) too small, need at least %lld blocks
>
> but if agcount was specified, and was too large, then say:
>
>> too many allocation groups for size = %lld
>> need at most %lld allocation groups
>
> but combining those two messages doesn't make sense to me.
>
Got it, Eric.
Regards,
Sheng-Hui
> -Eric
>
>
>> Signed-off-by: Wang Sheng-Hui <shhuiw@foxmail.com>
>> ---
>> mkfs/xfs_mkfs.c | 24 ++++++++----------------
>> 1 file changed, 8 insertions(+), 16 deletions(-)
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index e2a052d..7ec2556 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -740,22 +740,6 @@ validate_ag_geometry(
>> __uint64_t agsize,
>> __uint64_t agcount)
>> {
>> - if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
>> - fprintf(stderr,
>> - _("agsize (%lld blocks) too small, need at least %lld blocks\n"),
>> - (long long)agsize,
>> - (long long)XFS_AG_MIN_BLOCKS(blocklog));
>> - usage();
>> - }
>> -
>> - if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
>> - fprintf(stderr,
>> - _("agsize (%lld blocks) too big, maximum is %lld blocks\n"),
>> - (long long)agsize,
>> - (long long)XFS_AG_MAX_BLOCKS(blocklog));
>> - usage();
>> - }
>> -
>> if (agsize > dblocks) {
>> fprintf(stderr,
>> _("agsize (%lld blocks) too big, data area is %lld blocks\n"),
>> @@ -765,6 +749,10 @@ validate_ag_geometry(
>>
>> if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
>> fprintf(stderr,
>> + _("agsize (%lld blocks) too small, need at least %lld blocks\n"),
>> + (long long)agsize,
>> + (long long)XFS_AG_MIN_BLOCKS(blocklog));
>> + fprintf(stderr,
>> _("too many allocation groups for size = %lld\n"),
>> (long long)agsize);
>> fprintf(stderr, _("need at most %lld allocation groups\n"),
>> @@ -775,6 +763,10 @@ validate_ag_geometry(
>>
>> if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
>> fprintf(stderr,
>> + _("agsize (%lld blocks) too big, maximum is %lld blocks\n"),
>> + (long long)agsize,
>> + (long long)XFS_AG_MAX_BLOCKS(blocklog));
>> + fprintf(stderr,
>> _("too few allocation groups for size = %lld\n"), (long long)agsize);
>> fprintf(stderr,
>> _("need at least %lld allocation groups\n"),
>>
>
>
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-05-29 0:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-28 7:06 [PATCH] xfsprogs: merge the duplicated condition in xfs_mkfs.c/validate_ag_geometry Wang Sheng-Hui
2015-05-28 14:55 ` Eric Sandeen
2015-05-29 0:05 ` Wang Sheng-Hui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox