| Message ID | 20190828095619.9923-1-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] btrfs-progs: add BTRFS_DEV_ITEMS_OBJECTID in comment in print-tree | expand |
On 28.08.19 г. 12:56 ч., Anand Jain wrote: > So when searching for BTRFS_DEV_ITEMS_OBJECTID it hits. Albeit it is > defined same as BTRFS_ROOT_TREE_OBJECTID. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > print-tree.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/print-tree.c b/print-tree.c > index b31e515f8989..5832f3089e3d 100644 > --- a/print-tree.c > +++ b/print-tree.c > @@ -704,6 +704,7 @@ void print_objectid(FILE *stream, u64 objectid, u8 type) > } > > switch (objectid) { > + /* BTRFS_DEV_ITEMS_OBJECTID */ That comment looks really cryptic to someone just looking at the code. Adding case BTRFS_DEV_ITEMS_OBJECTID: is better. > case BTRFS_ROOT_TREE_OBJECTID: > if (type == BTRFS_DEV_ITEM_KEY) > fprintf(stream, "DEV_ITEMS"); >
On Wed, Aug 28, 2019 at 04:01:04PM +0300, Nikolay Borisov wrote: > > > On 28.08.19 г. 12:56 ч., Anand Jain wrote: > > So when searching for BTRFS_DEV_ITEMS_OBJECTID it hits. Albeit it is > > defined same as BTRFS_ROOT_TREE_OBJECTID. > > > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > > --- > > print-tree.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/print-tree.c b/print-tree.c > > index b31e515f8989..5832f3089e3d 100644 > > --- a/print-tree.c > > +++ b/print-tree.c > > @@ -704,6 +704,7 @@ void print_objectid(FILE *stream, u64 objectid, u8 type) > > } > > > > switch (objectid) { > > + /* BTRFS_DEV_ITEMS_OBJECTID */ > > That comment looks really cryptic to someone just looking at the code. > Adding case BTRFS_DEV_ITEMS_OBJECTID: is better. > > case BTRFS_ROOT_TREE_OBJECTID: Both constants have the same value so they can't be in one switch, but yeah the comment should be a bit more verbose, just the constant name is bringing more questions than answers.
> On 28 Aug 2019, at 9:01 PM, Nikolay Borisov <nborisov@suse.com> wrote: > > > > On 28.08.19 г. 12:56 ч., Anand Jain wrote: >> So when searching for BTRFS_DEV_ITEMS_OBJECTID it hits. Albeit it is >> defined same as BTRFS_ROOT_TREE_OBJECTID. >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> >> --- >> print-tree.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/print-tree.c b/print-tree.c >> index b31e515f8989..5832f3089e3d 100644 >> --- a/print-tree.c >> +++ b/print-tree.c >> @@ -704,6 +704,7 @@ void print_objectid(FILE *stream, u64 objectid, u8 type) >> } >> >> switch (objectid) { >> + /* BTRFS_DEV_ITEMS_OBJECTID */ > > That comment looks really cryptic to someone just looking at the code. > Adding case BTRFS_DEV_ITEMS_OBJECTID: is better. > Both of them defined with the same value (we find which object by type). So only one of it can be in the case. #define BTRFS_DEV_ITEMS_OBJECTID 1ULL :: #define BTRFS_ROOT_TREE_OBJECTID 1ULL Thanks, Anand >> case BTRFS_ROOT_TREE_OBJECTID: >> if (type == BTRFS_DEV_ITEM_KEY) >> fprintf(stream, "DEV_ITEMS"); >>
> On 28 Aug 2019, at 9:24 PM, David Sterba <dsterba@suse.cz> wrote: > > On Wed, Aug 28, 2019 at 04:01:04PM +0300, Nikolay Borisov wrote: >> >> >> On 28.08.19 г. 12:56 ч., Anand Jain wrote: >>> So when searching for BTRFS_DEV_ITEMS_OBJECTID it hits. Albeit it is >>> defined same as BTRFS_ROOT_TREE_OBJECTID. >>> >>> Signed-off-by: Anand Jain <anand.jain@oracle.com> >>> --- >>> print-tree.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/print-tree.c b/print-tree.c >>> index b31e515f8989..5832f3089e3d 100644 >>> --- a/print-tree.c >>> +++ b/print-tree.c >>> @@ -704,6 +704,7 @@ void print_objectid(FILE *stream, u64 objectid, u8 type) >>> } >>> >>> switch (objectid) { >>> + /* BTRFS_DEV_ITEMS_OBJECTID */ >> >> That comment looks really cryptic to someone just looking at the code. >> Adding case BTRFS_DEV_ITEMS_OBJECTID: is better. > >>> case BTRFS_ROOT_TREE_OBJECTID: > > Both constants have the same value so they can't be in one switch, but > yeah the comment should be a bit more verbose, just the constant name is > bringing more questions than answers. Oh. Hmm. Let me try. Thanks, Anand
On Wed, Aug 28, 2019 at 05:56:18PM +0800, Anand Jain wrote: > So when searching for BTRFS_DEV_ITEMS_OBJECTID it hits. Albeit it is > defined same as BTRFS_ROOT_TREE_OBJECTID. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> 1-2 applied, thanks.
diff --git a/print-tree.c b/print-tree.c index b31e515f8989..5832f3089e3d 100644 --- a/print-tree.c +++ b/print-tree.c @@ -704,6 +704,7 @@ void print_objectid(FILE *stream, u64 objectid, u8 type) } switch (objectid) { + /* BTRFS_DEV_ITEMS_OBJECTID */ case BTRFS_ROOT_TREE_OBJECTID: if (type == BTRFS_DEV_ITEM_KEY) fprintf(stream, "DEV_ITEMS");
So when searching for BTRFS_DEV_ITEMS_OBJECTID it hits. Albeit it is defined same as BTRFS_ROOT_TREE_OBJECTID. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- print-tree.c | 1 + 1 file changed, 1 insertion(+)