| Message ID | 1419470195-2889-2-git-send-email-guihc.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 2014/12/25 10:16, Gui Hecheng wrote: > Now, if exec: > # btrfs-debug-tree <mount_point> > it echos: > : Superblock bytenr is larger than device size > > But it is quite misleading, because it is a valid btrfs. > In this case, we should tell the developer to provide a block device. > > After apply: > : '<mount_point>' is not a block device > : 'usage: btrfs-debug-tree [options] device > > Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com> > --- > btrfs-debug-tree.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/btrfs-debug-tree.c b/btrfs-debug-tree.c > index e46500d..7f079a9 100644 > --- a/btrfs-debug-tree.c > +++ b/btrfs-debug-tree.c > @@ -179,6 +179,12 @@ int main(int ac, char **av) > if (check_argc_exact(ac, 1)) > print_usage(); > > + ret = check_arg_type(av[optind]); > + if (ret != BTRFS_ARG_BLKDEV) { > + fprintf(stderr, "'%s' is not a block device\n", av[optind]); fprintf(stderr, "ERROR: '%s' is ...)" is better since other error messages in btrfs-progs have this convention. Reviewed-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com> Thanks, Satoru > + print_usage(); > + } > + > info = open_ctree_fs_info(av[optind], 0, 0, OPEN_CTREE_PARTIAL); > if (!info) { > fprintf(stderr, "unable to open %s\n", av[optind]); > -- 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
On Thu, 2014-12-25 at 15:49 +0900, Satoru Takeuchi wrote: > On 2014/12/25 10:16, Gui Hecheng wrote: > > Now, if exec: > > # btrfs-debug-tree <mount_point> > > it echos: > > : Superblock bytenr is larger than device size > > > > But it is quite misleading, because it is a valid btrfs. > > In this case, we should tell the developer to provide a block device. > > > > After apply: > > : '<mount_point>' is not a block device > > : 'usage: btrfs-debug-tree [options] device > > > > Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com> > > --- > > btrfs-debug-tree.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/btrfs-debug-tree.c b/btrfs-debug-tree.c > > index e46500d..7f079a9 100644 > > --- a/btrfs-debug-tree.c > > +++ b/btrfs-debug-tree.c > > @@ -179,6 +179,12 @@ int main(int ac, char **av) > > if (check_argc_exact(ac, 1)) > > print_usage(); > > > > + ret = check_arg_type(av[optind]); > > + if (ret != BTRFS_ARG_BLKDEV) { > > + fprintf(stderr, "'%s' is not a block device\n", av[optind]); > > fprintf(stderr, "ERROR: '%s' is ...)" is better since > other error messages in btrfs-progs have this convention. Yes, it is good to make it clear about the "ERROR", but since there is no "ERROR" word throughout the source file, I think I would make all error prompts to start with the "ERROR" word globally in the progs in another patch. Thanks, Gui > Reviewed-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com> > > Thanks, > Satoru > > > + print_usage(); > > + } > > + > > info = open_ctree_fs_info(av[optind], 0, 0, OPEN_CTREE_PARTIAL); > > if (!info) { > > fprintf(stderr, "unable to open %s\n", av[optind]); > > > -- 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
On Thu, Dec 25, 2014 at 09:16:35AM +0800, Gui Hecheng wrote: > Now, if exec: > # btrfs-debug-tree <mount_point> > it echos: > : Superblock bytenr is larger than device size > > But it is quite misleading, because it is a valid btrfs. > In this case, we should tell the developer to provide a block device. > > After apply: > : '<mount_point>' is not a block device > : 'usage: btrfs-debug-tree [options] device > > Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com> > --- > btrfs-debug-tree.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/btrfs-debug-tree.c b/btrfs-debug-tree.c > index e46500d..7f079a9 100644 > --- a/btrfs-debug-tree.c > +++ b/btrfs-debug-tree.c > @@ -179,6 +179,12 @@ int main(int ac, char **av) > if (check_argc_exact(ac, 1)) > print_usage(); > > + ret = check_arg_type(av[optind]); > + if (ret != BTRFS_ARG_BLKDEV) { > + fprintf(stderr, "'%s' is not a block device\n", av[optind]); > + print_usage(); The current widespread pattern is to print_usage() after most errors in commandline arguments but I find it quite annoying. The help is always available under --help for each command. As the bugfix is good I'm going to apply it and replace it with exit(). We can start removing misues of print_usage() from the codebase in the next dev cycle. -- 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
diff --git a/btrfs-debug-tree.c b/btrfs-debug-tree.c index e46500d..7f079a9 100644 --- a/btrfs-debug-tree.c +++ b/btrfs-debug-tree.c @@ -179,6 +179,12 @@ int main(int ac, char **av) if (check_argc_exact(ac, 1)) print_usage(); + ret = check_arg_type(av[optind]); + if (ret != BTRFS_ARG_BLKDEV) { + fprintf(stderr, "'%s' is not a block device\n", av[optind]); + print_usage(); + } + info = open_ctree_fs_info(av[optind], 0, 0, OPEN_CTREE_PARTIAL); if (!info) { fprintf(stderr, "unable to open %s\n", av[optind]);
Now, if exec: # btrfs-debug-tree <mount_point> it echos: : Superblock bytenr is larger than device size But it is quite misleading, because it is a valid btrfs. In this case, we should tell the developer to provide a block device. After apply: : '<mount_point>' is not a block device : 'usage: btrfs-debug-tree [options] device Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com> --- btrfs-debug-tree.c | 6 ++++++ 1 file changed, 6 insertions(+)