diff mbox series

[v12,12/17] btrfs: send: fix maximum command numbering

Message ID bc324fbf99e8a792719da7bb96f5dcf4964904de.1637179348.git.osandov@fb.com (mailing list archive)
State New, archived
Headers show
Series btrfs: add ioctls and send/receive support for reading/writing compressed data | expand

Commit Message

Omar Sandoval Nov. 17, 2021, 8:19 p.m. UTC 
From: Omar Sandoval <osandov@fb.com>

Commit e77fbf990316 ("btrfs: send: prepare for v2 protocol") added
_BTRFS_SEND_C_MAX_V* macros equal to the maximum command number for the
version plus 1, but as written this creates gaps in the number space.
The maximum command number is currently 22, and __BTRFS_SEND_C_MAX_V1 is
accordingly 23. But then __BTRFS_SEND_C_MAX_V2 is 24, suggesting that v2
has a command numbered 23, and __BTRFS_SEND_C_MAX is 25, suggesting that
23 and 24 are valid commands.

Instead, let's explicitly set BTRFS_SEND_C_MAX_V* to the maximum command
number. This requires repeating the command name, but it has a clearer
meaning and avoids gaps. It also doesn't require updating
__BTRFS_SEND_C_MAX for every new version.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/btrfs/send.c | 4 ++--
 fs/btrfs/send.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

David Sterba Nov. 18, 2021, 2:23 p.m. UTC | #1 
On Wed, Nov 17, 2021 at 12:19:22PM -0800, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> Commit e77fbf990316 ("btrfs: send: prepare for v2 protocol") added
> _BTRFS_SEND_C_MAX_V* macros equal to the maximum command number for the
> version plus 1, but as written this creates gaps in the number space.
> The maximum command number is currently 22, and __BTRFS_SEND_C_MAX_V1 is
> accordingly 23. But then __BTRFS_SEND_C_MAX_V2 is 24, suggesting that v2
> has a command numbered 23, and __BTRFS_SEND_C_MAX is 25, suggesting that
> 23 and 24 are valid commands.

The MAX definitions have the __ prefix so they're private and not meant
to be used as proper commands, so nothing should suggest there are any
commands with numbers 23 to 25 in the example.

> Instead, let's explicitly set BTRFS_SEND_C_MAX_V* to the maximum command
> number. This requires repeating the command name, but it has a clearer
> meaning and avoids gaps. It also doesn't require updating
> __BTRFS_SEND_C_MAX for every new version.

It's probably a matter of taste, I'd intentionally avoid the pattern
above, ie. repeating the previous command to define max.

> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -316,8 +316,8 @@ __maybe_unused
>  static bool proto_cmd_ok(const struct send_ctx *sctx, int cmd)
>  {
>  	switch (sctx->proto) {
> -	case 1:	 return cmd < __BTRFS_SEND_C_MAX_V1;
> -	case 2:	 return cmd < __BTRFS_SEND_C_MAX_V2;
> +	case 1:	 return cmd <= BTRFS_SEND_C_MAX_V1;
> +	case 2:	 return cmd <= BTRFS_SEND_C_MAX_V2;

This seems to be the only practical difference, < or <= .
Omar Sandoval Nov. 18, 2021, 6:54 p.m. UTC | #2 
On Thu, Nov 18, 2021 at 03:23:59PM +0100, David Sterba wrote:
> On Wed, Nov 17, 2021 at 12:19:22PM -0800, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > Commit e77fbf990316 ("btrfs: send: prepare for v2 protocol") added
> > _BTRFS_SEND_C_MAX_V* macros equal to the maximum command number for the
> > version plus 1, but as written this creates gaps in the number space.
> > The maximum command number is currently 22, and __BTRFS_SEND_C_MAX_V1 is
> > accordingly 23. But then __BTRFS_SEND_C_MAX_V2 is 24, suggesting that v2
> > has a command numbered 23, and __BTRFS_SEND_C_MAX is 25, suggesting that
> > 23 and 24 are valid commands.
> 
> The MAX definitions have the __ prefix so they're private and not meant
> to be used as proper commands, so nothing should suggest there are any
> commands with numbers 23 to 25 in the example.
> 
> > Instead, let's explicitly set BTRFS_SEND_C_MAX_V* to the maximum command
> > number. This requires repeating the command name, but it has a clearer
> > meaning and avoids gaps. It also doesn't require updating
> > __BTRFS_SEND_C_MAX for every new version.
> 
> It's probably a matter of taste, I'd intentionally avoid the pattern
> above, ie. repeating the previous command to define max.
> 
> > --- a/fs/btrfs/send.c
> > +++ b/fs/btrfs/send.c
> > @@ -316,8 +316,8 @@ __maybe_unused
> >  static bool proto_cmd_ok(const struct send_ctx *sctx, int cmd)
> >  {
> >  	switch (sctx->proto) {
> > -	case 1:	 return cmd < __BTRFS_SEND_C_MAX_V1;
> > -	case 2:	 return cmd < __BTRFS_SEND_C_MAX_V2;
> > +	case 1:	 return cmd <= BTRFS_SEND_C_MAX_V1;
> > +	case 2:	 return cmd <= BTRFS_SEND_C_MAX_V2;
> 
> This seems to be the only practical difference, < or <= .

There is another practical difference, which is more significant in my
opinion: the linear style creates "gaps" in the valid commands. Consider
this, with explicit values added for clarity:

enum btrfs_send_cmd {
        BTRFS_SEND_C_UNSPEC = 0,

        /* Version 1 */
        BTRFS_SEND_C_SUBVOL = 1,
        BTRFS_SEND_C_SNAPSHOT = 2,

        BTRFS_SEND_C_MKFILE = 3,
        BTRFS_SEND_C_MKDIR = 4,
        BTRFS_SEND_C_MKNOD = 5,
        BTRFS_SEND_C_MKFIFO = 6,
        BTRFS_SEND_C_MKSOCK = 7,
        BTRFS_SEND_C_SYMLINK = 8,

        BTRFS_SEND_C_RENAME = 9,
        BTRFS_SEND_C_LINK = 10,
        BTRFS_SEND_C_UNLINK = 11,
        BTRFS_SEND_C_RMDIR = 12,

        BTRFS_SEND_C_SET_XATTR = 13,
        BTRFS_SEND_C_REMOVE_XATTR = 14,

        BTRFS_SEND_C_WRITE = 15,
        BTRFS_SEND_C_CLONE = 16,

        BTRFS_SEND_C_TRUNCATE = 17,
        BTRFS_SEND_C_CHMOD = 18,
        BTRFS_SEND_C_CHOWN = 19,
        BTRFS_SEND_C_UTIMES = 20,

        BTRFS_SEND_C_END = 21,
        BTRFS_SEND_C_UPDATE_EXTENT = 22,
        __BTRFS_SEND_C_MAX_V1 = 23,

        /* Version 2 */
        BTRFS_SEND_C_FALLOCATE = 24,
        BTRFS_SEND_C_SETFLAGS = 25,
        BTRFS_SEND_C_ENCODED_WRITE = 26,
        __BTRFS_SEND_C_MAX_V2 = 27,

        /* End */
        __BTRFS_SEND_C_MAX = 28,
};
#define BTRFS_SEND_C_MAX (__BTRFS_SEND_C_MAX - 1) /* 27 */

Notice that BTRFS_SEND_C_UPDATE_EXTENT is 22 and the next valid command
is BTRFS_SEND_C_FALLOCATE, which is 24. So 23 does not correspond to an
actual command; it's a "gap". This is somewhat cosmetic, but it's an
ugly wart in the protocol.

Also consider something indexing on the command number, like the
cmd_send_size thing I got rid of in the previous patch:

	u64 cmd_send_size[BTRFS_SEND_C_MAX + 1]

Indices 23 and 27 are wasted. It's only 16 bytes in this case, which
doesn't matter practically, but it's unpleasant.

Maybe you were aware of this and fine with it, in which case we can drop
this change. But I think the name repetition is less ugly than the gaps.
Omar Sandoval Dec. 9, 2021, 6:08 p.m. UTC | #3 
On Thu, Nov 18, 2021 at 10:54:16AM -0800, Omar Sandoval wrote:
> On Thu, Nov 18, 2021 at 03:23:59PM +0100, David Sterba wrote:
> > On Wed, Nov 17, 2021 at 12:19:22PM -0800, Omar Sandoval wrote:
> > > From: Omar Sandoval <osandov@fb.com>
> > > 
> > > Commit e77fbf990316 ("btrfs: send: prepare for v2 protocol") added
> > > _BTRFS_SEND_C_MAX_V* macros equal to the maximum command number for the
> > > version plus 1, but as written this creates gaps in the number space.
> > > The maximum command number is currently 22, and __BTRFS_SEND_C_MAX_V1 is
> > > accordingly 23. But then __BTRFS_SEND_C_MAX_V2 is 24, suggesting that v2
> > > has a command numbered 23, and __BTRFS_SEND_C_MAX is 25, suggesting that
> > > 23 and 24 are valid commands.
> > 
> > The MAX definitions have the __ prefix so they're private and not meant
> > to be used as proper commands, so nothing should suggest there are any
> > commands with numbers 23 to 25 in the example.
> > 
> > > Instead, let's explicitly set BTRFS_SEND_C_MAX_V* to the maximum command
> > > number. This requires repeating the command name, but it has a clearer
> > > meaning and avoids gaps. It also doesn't require updating
> > > __BTRFS_SEND_C_MAX for every new version.
> > 
> > It's probably a matter of taste, I'd intentionally avoid the pattern
> > above, ie. repeating the previous command to define max.
> > 
> > > --- a/fs/btrfs/send.c
> > > +++ b/fs/btrfs/send.c
> > > @@ -316,8 +316,8 @@ __maybe_unused
> > >  static bool proto_cmd_ok(const struct send_ctx *sctx, int cmd)
> > >  {
> > >  	switch (sctx->proto) {
> > > -	case 1:	 return cmd < __BTRFS_SEND_C_MAX_V1;
> > > -	case 2:	 return cmd < __BTRFS_SEND_C_MAX_V2;
> > > +	case 1:	 return cmd <= BTRFS_SEND_C_MAX_V1;
> > > +	case 2:	 return cmd <= BTRFS_SEND_C_MAX_V2;
> > 
> > This seems to be the only practical difference, < or <= .
> 
> There is another practical difference, which is more significant in my
> opinion: the linear style creates "gaps" in the valid commands. Consider
> this, with explicit values added for clarity:
> 
> enum btrfs_send_cmd {
>         BTRFS_SEND_C_UNSPEC = 0,
> 
>         /* Version 1 */
>         BTRFS_SEND_C_SUBVOL = 1,
>         BTRFS_SEND_C_SNAPSHOT = 2,
> 
>         BTRFS_SEND_C_MKFILE = 3,
>         BTRFS_SEND_C_MKDIR = 4,
>         BTRFS_SEND_C_MKNOD = 5,
>         BTRFS_SEND_C_MKFIFO = 6,
>         BTRFS_SEND_C_MKSOCK = 7,
>         BTRFS_SEND_C_SYMLINK = 8,
> 
>         BTRFS_SEND_C_RENAME = 9,
>         BTRFS_SEND_C_LINK = 10,
>         BTRFS_SEND_C_UNLINK = 11,
>         BTRFS_SEND_C_RMDIR = 12,
> 
>         BTRFS_SEND_C_SET_XATTR = 13,
>         BTRFS_SEND_C_REMOVE_XATTR = 14,
> 
>         BTRFS_SEND_C_WRITE = 15,
>         BTRFS_SEND_C_CLONE = 16,
> 
>         BTRFS_SEND_C_TRUNCATE = 17,
>         BTRFS_SEND_C_CHMOD = 18,
>         BTRFS_SEND_C_CHOWN = 19,
>         BTRFS_SEND_C_UTIMES = 20,
> 
>         BTRFS_SEND_C_END = 21,
>         BTRFS_SEND_C_UPDATE_EXTENT = 22,
>         __BTRFS_SEND_C_MAX_V1 = 23,
> 
>         /* Version 2 */
>         BTRFS_SEND_C_FALLOCATE = 24,
>         BTRFS_SEND_C_SETFLAGS = 25,
>         BTRFS_SEND_C_ENCODED_WRITE = 26,
>         __BTRFS_SEND_C_MAX_V2 = 27,
> 
>         /* End */
>         __BTRFS_SEND_C_MAX = 28,
> };
> #define BTRFS_SEND_C_MAX (__BTRFS_SEND_C_MAX - 1) /* 27 */
> 
> Notice that BTRFS_SEND_C_UPDATE_EXTENT is 22 and the next valid command
> is BTRFS_SEND_C_FALLOCATE, which is 24. So 23 does not correspond to an
> actual command; it's a "gap". This is somewhat cosmetic, but it's an
> ugly wart in the protocol.
> 
> Also consider something indexing on the command number, like the
> cmd_send_size thing I got rid of in the previous patch:
> 
> 	u64 cmd_send_size[BTRFS_SEND_C_MAX + 1]
> 
> Indices 23 and 27 are wasted. It's only 16 bytes in this case, which
> doesn't matter practically, but it's unpleasant.
> 
> Maybe you were aware of this and fine with it, in which case we can drop
> this change. But I think the name repetition is less ugly than the gaps.

Ping. Please let me know how you'd like me to proceed on this issue and
my other replies. Thanks!
Omar Sandoval Jan. 4, 2022, 7:05 p.m. UTC | #4 
On Thu, Dec 09, 2021 at 10:08:02AM -0800, Omar Sandoval wrote:
> On Thu, Nov 18, 2021 at 10:54:16AM -0800, Omar Sandoval wrote:
> > On Thu, Nov 18, 2021 at 03:23:59PM +0100, David Sterba wrote:
> > > On Wed, Nov 17, 2021 at 12:19:22PM -0800, Omar Sandoval wrote:
> > > > From: Omar Sandoval <osandov@fb.com>
> > > > 
> > > > Commit e77fbf990316 ("btrfs: send: prepare for v2 protocol") added
> > > > _BTRFS_SEND_C_MAX_V* macros equal to the maximum command number for the
> > > > version plus 1, but as written this creates gaps in the number space.
> > > > The maximum command number is currently 22, and __BTRFS_SEND_C_MAX_V1 is
> > > > accordingly 23. But then __BTRFS_SEND_C_MAX_V2 is 24, suggesting that v2
> > > > has a command numbered 23, and __BTRFS_SEND_C_MAX is 25, suggesting that
> > > > 23 and 24 are valid commands.
> > > 
> > > The MAX definitions have the __ prefix so they're private and not meant
> > > to be used as proper commands, so nothing should suggest there are any
> > > commands with numbers 23 to 25 in the example.
> > > 
> > > > Instead, let's explicitly set BTRFS_SEND_C_MAX_V* to the maximum command
> > > > number. This requires repeating the command name, but it has a clearer
> > > > meaning and avoids gaps. It also doesn't require updating
> > > > __BTRFS_SEND_C_MAX for every new version.
> > > 
> > > It's probably a matter of taste, I'd intentionally avoid the pattern
> > > above, ie. repeating the previous command to define max.
> > > 
> > > > --- a/fs/btrfs/send.c
> > > > +++ b/fs/btrfs/send.c
> > > > @@ -316,8 +316,8 @@ __maybe_unused
> > > >  static bool proto_cmd_ok(const struct send_ctx *sctx, int cmd)
> > > >  {
> > > >  	switch (sctx->proto) {
> > > > -	case 1:	 return cmd < __BTRFS_SEND_C_MAX_V1;
> > > > -	case 2:	 return cmd < __BTRFS_SEND_C_MAX_V2;
> > > > +	case 1:	 return cmd <= BTRFS_SEND_C_MAX_V1;
> > > > +	case 2:	 return cmd <= BTRFS_SEND_C_MAX_V2;
> > > 
> > > This seems to be the only practical difference, < or <= .
> > 
> > There is another practical difference, which is more significant in my
> > opinion: the linear style creates "gaps" in the valid commands. Consider
> > this, with explicit values added for clarity:
> > 
> > enum btrfs_send_cmd {
> >         BTRFS_SEND_C_UNSPEC = 0,
> > 
> >         /* Version 1 */
> >         BTRFS_SEND_C_SUBVOL = 1,
> >         BTRFS_SEND_C_SNAPSHOT = 2,
> > 
> >         BTRFS_SEND_C_MKFILE = 3,
> >         BTRFS_SEND_C_MKDIR = 4,
> >         BTRFS_SEND_C_MKNOD = 5,
> >         BTRFS_SEND_C_MKFIFO = 6,
> >         BTRFS_SEND_C_MKSOCK = 7,
> >         BTRFS_SEND_C_SYMLINK = 8,
> > 
> >         BTRFS_SEND_C_RENAME = 9,
> >         BTRFS_SEND_C_LINK = 10,
> >         BTRFS_SEND_C_UNLINK = 11,
> >         BTRFS_SEND_C_RMDIR = 12,
> > 
> >         BTRFS_SEND_C_SET_XATTR = 13,
> >         BTRFS_SEND_C_REMOVE_XATTR = 14,
> > 
> >         BTRFS_SEND_C_WRITE = 15,
> >         BTRFS_SEND_C_CLONE = 16,
> > 
> >         BTRFS_SEND_C_TRUNCATE = 17,
> >         BTRFS_SEND_C_CHMOD = 18,
> >         BTRFS_SEND_C_CHOWN = 19,
> >         BTRFS_SEND_C_UTIMES = 20,
> > 
> >         BTRFS_SEND_C_END = 21,
> >         BTRFS_SEND_C_UPDATE_EXTENT = 22,
> >         __BTRFS_SEND_C_MAX_V1 = 23,
> > 
> >         /* Version 2 */
> >         BTRFS_SEND_C_FALLOCATE = 24,
> >         BTRFS_SEND_C_SETFLAGS = 25,
> >         BTRFS_SEND_C_ENCODED_WRITE = 26,
> >         __BTRFS_SEND_C_MAX_V2 = 27,
> > 
> >         /* End */
> >         __BTRFS_SEND_C_MAX = 28,
> > };
> > #define BTRFS_SEND_C_MAX (__BTRFS_SEND_C_MAX - 1) /* 27 */
> > 
> > Notice that BTRFS_SEND_C_UPDATE_EXTENT is 22 and the next valid command
> > is BTRFS_SEND_C_FALLOCATE, which is 24. So 23 does not correspond to an
> > actual command; it's a "gap". This is somewhat cosmetic, but it's an
> > ugly wart in the protocol.
> > 
> > Also consider something indexing on the command number, like the
> > cmd_send_size thing I got rid of in the previous patch:
> > 
> > 	u64 cmd_send_size[BTRFS_SEND_C_MAX + 1]
> > 
> > Indices 23 and 27 are wasted. It's only 16 bytes in this case, which
> > doesn't matter practically, but it's unpleasant.
> > 
> > Maybe you were aware of this and fine with it, in which case we can drop
> > this change. But I think the name repetition is less ugly than the gaps.
> 
> Ping. Please let me know how you'd like me to proceed on this issue and
> my other replies. Thanks!

New year, new ping. Thanks!
David Sterba Jan. 24, 2022, 10:40 p.m. UTC | #5 
On Thu, Nov 18, 2021 at 10:54:16AM -0800, Omar Sandoval wrote:
> On Thu, Nov 18, 2021 at 03:23:59PM +0100, David Sterba wrote:
> > On Wed, Nov 17, 2021 at 12:19:22PM -0800, Omar Sandoval wrote:
> > > From: Omar Sandoval <osandov@fb.com>
> > > 
> > > Commit e77fbf990316 ("btrfs: send: prepare for v2 protocol") added
> > > _BTRFS_SEND_C_MAX_V* macros equal to the maximum command number for the
> > > version plus 1, but as written this creates gaps in the number space.
> > > The maximum command number is currently 22, and __BTRFS_SEND_C_MAX_V1 is
> > > accordingly 23. But then __BTRFS_SEND_C_MAX_V2 is 24, suggesting that v2
> > > has a command numbered 23, and __BTRFS_SEND_C_MAX is 25, suggesting that
> > > 23 and 24 are valid commands.
> > 
> > The MAX definitions have the __ prefix so they're private and not meant
> > to be used as proper commands, so nothing should suggest there are any
> > commands with numbers 23 to 25 in the example.
> > 
> > > Instead, let's explicitly set BTRFS_SEND_C_MAX_V* to the maximum command
> > > number. This requires repeating the command name, but it has a clearer
> > > meaning and avoids gaps. It also doesn't require updating
> > > __BTRFS_SEND_C_MAX for every new version.
> > 
> > It's probably a matter of taste, I'd intentionally avoid the pattern
> > above, ie. repeating the previous command to define max.
> > 
> > > --- a/fs/btrfs/send.c
> > > +++ b/fs/btrfs/send.c
> > > @@ -316,8 +316,8 @@ __maybe_unused
> > >  static bool proto_cmd_ok(const struct send_ctx *sctx, int cmd)
> > >  {
> > >  	switch (sctx->proto) {
> > > -	case 1:	 return cmd < __BTRFS_SEND_C_MAX_V1;
> > > -	case 2:	 return cmd < __BTRFS_SEND_C_MAX_V2;
> > > +	case 1:	 return cmd <= BTRFS_SEND_C_MAX_V1;
> > > +	case 2:	 return cmd <= BTRFS_SEND_C_MAX_V2;
> > 
> > This seems to be the only practical difference, < or <= .
> 
> There is another practical difference, which is more significant in my
> opinion: the linear style creates "gaps" in the valid commands. Consider
> this, with explicit values added for clarity:
> 
> enum btrfs_send_cmd {
>         BTRFS_SEND_C_UNSPEC = 0,
> 
>         /* Version 1 */
>         BTRFS_SEND_C_SUBVOL = 1,
>         BTRFS_SEND_C_SNAPSHOT = 2,
> 
>         BTRFS_SEND_C_MKFILE = 3,
>         BTRFS_SEND_C_MKDIR = 4,
>         BTRFS_SEND_C_MKNOD = 5,
>         BTRFS_SEND_C_MKFIFO = 6,
>         BTRFS_SEND_C_MKSOCK = 7,
>         BTRFS_SEND_C_SYMLINK = 8,
> 
>         BTRFS_SEND_C_RENAME = 9,
>         BTRFS_SEND_C_LINK = 10,
>         BTRFS_SEND_C_UNLINK = 11,
>         BTRFS_SEND_C_RMDIR = 12,
> 
>         BTRFS_SEND_C_SET_XATTR = 13,
>         BTRFS_SEND_C_REMOVE_XATTR = 14,
> 
>         BTRFS_SEND_C_WRITE = 15,
>         BTRFS_SEND_C_CLONE = 16,
> 
>         BTRFS_SEND_C_TRUNCATE = 17,
>         BTRFS_SEND_C_CHMOD = 18,
>         BTRFS_SEND_C_CHOWN = 19,
>         BTRFS_SEND_C_UTIMES = 20,
> 
>         BTRFS_SEND_C_END = 21,
>         BTRFS_SEND_C_UPDATE_EXTENT = 22,
>         __BTRFS_SEND_C_MAX_V1 = 23,
> 
>         /* Version 2 */
>         BTRFS_SEND_C_FALLOCATE = 24,
>         BTRFS_SEND_C_SETFLAGS = 25,
>         BTRFS_SEND_C_ENCODED_WRITE = 26,
>         __BTRFS_SEND_C_MAX_V2 = 27,
> 
>         /* End */
>         __BTRFS_SEND_C_MAX = 28,
> };
> #define BTRFS_SEND_C_MAX (__BTRFS_SEND_C_MAX - 1) /* 27 */

So as a compromise to avoid gaps and also repeating the last command name in
the definition, let's do it in a similar way as in your example,
explicit numbering of the commands, so the number will be repated for
the MAX constants.
diff mbox series

Patch

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 500b866ede43..450c873684e8 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -316,8 +316,8 @@  __maybe_unused
 static bool proto_cmd_ok(const struct send_ctx *sctx, int cmd)
 {
 	switch (sctx->proto) {
-	case 1:	 return cmd < __BTRFS_SEND_C_MAX_V1;
-	case 2:	 return cmd < __BTRFS_SEND_C_MAX_V2;
+	case 1:	 return cmd <= BTRFS_SEND_C_MAX_V1;
+	case 2:	 return cmd <= BTRFS_SEND_C_MAX_V2;
 	default: return false;
 	}
 }
diff --git a/fs/btrfs/send.h b/fs/btrfs/send.h
index 23bcefc84e49..59a4be3b09cd 100644
--- a/fs/btrfs/send.h
+++ b/fs/btrfs/send.h
@@ -77,10 +77,10 @@  enum btrfs_send_cmd {
 
 	BTRFS_SEND_C_END,
 	BTRFS_SEND_C_UPDATE_EXTENT,
-	__BTRFS_SEND_C_MAX_V1,
+	BTRFS_SEND_C_MAX_V1 = BTRFS_SEND_C_UPDATE_EXTENT,
 
 	/* Version 2 */
-	__BTRFS_SEND_C_MAX_V2,
+	BTRFS_SEND_C_MAX_V2 = BTRFS_SEND_C_MAX_V1,
 
 	/* End */
 	__BTRFS_SEND_C_MAX,