Fixed comments

microsoft · Oct 19, 2024 · 4286f97 · 4286f97
1 parent 7103c60
commit 4286f97
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 11 deletions.
diff --git a/libs/server/ACL/CommandPermissionSet.cs b/libs/server/ACL/CommandPermissionSet.cs
@@ -167,10 +167,9 @@ public bool IsEquivalentTo(CommandPermissionSet other)
         /// </summary>
         private static ushort GetCommandListLength()
         {
-            int maxCommandValue = (int)Enum.GetValues<RespCommand>().Where(static cmd => cmd != RespCommand.NONE && cmd != RespCommand.INVALID).Max();
-
-            int neededBits = maxCommandValue + 1;
-            int neededULongs = neededBits / 64;
+            // # of bits needed to represent all valid commands
+            var neededBits = (ushort)RespCommandExtensions.LastValidCommand + 1;
+            var neededULongs = neededBits / 64;
 
             if ((neededBits % 64) != 0)
             {

diff --git a/libs/server/Resp/Bitmap/BitmapManagerBitfield.cs b/libs/server/Resp/Bitmap/BitmapManagerBitfield.cs
@@ -459,7 +459,7 @@ public static (long, bool) BitFieldExecute(BitFieldCmdArgs args, byte* value, in
         {
             var bitCount = (byte)(args.typeInfo & 0x7F);
 
-            switch ((RespCommand)args.secondaryCommand)
+            switch (args.secondaryCommand)
             {
                 case RespCommand.SET:
                     return SetBitfieldValue(value, valLen, args.offset, bitCount, args.typeInfo, args.value, args.overflowType);

diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs
@@ -303,7 +303,10 @@ public enum RespCommand : ushort
         HELLO,
         QUIT, // Note: Update IsNoAuth if adding new no-auth commands after this
 
-        INVALID,
+        // Max value of this enum (not including INVALID) will determine the size of RespCommand.AofIndependentBitLookup and CommandPermissionSet._commandList,
+        // so avoid manually setting high values unless necessary
+
+        INVALID = 0xFFFF,
     }
 
     /// <summary>
@@ -365,12 +368,12 @@ public static class RespCommandExtensions
         // The static ctor maybe expensive but it is only ever run once, and doesn't interfere with common path
         static RespCommandExtensions()
         {
-            var commands = Enum.GetValues<RespCommand>();
-            var maxCommandValue = (ushort)commands.Max();
-            var lookupTableSize = ((maxCommandValue + 1) / 64) + ((maxCommandValue + 1) % 64 == 0 ? 0 : 1);
+            // # of bits needed to represent all valid commands
+            var maxBitsNeeded = (ushort)LastValidCommand + 1;
+            var lookupTableSize = (maxBitsNeeded / 64) + (maxBitsNeeded % 64 == 0 ? 0 : 1);
             AofIndependentBitLookup = new ulong[lookupTableSize];
 
-            foreach (var cmd in commands)
+            foreach (var cmd in Enum.GetValues<RespCommand>())
             {
                 if (Array.IndexOf(AofIndependentCommands, cmd) == -1)
                     continue;
@@ -390,6 +393,8 @@ static RespCommandExtensions()
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool IsAofIndependent(this RespCommand cmd)
         {
+            Debug.Assert(cmd <= LastValidCommand);
+
             // check if cmd maps to a bit vec that was set back when static ctor was run
             int bitIdxToUse = (int)cmd / sizeOfLong;
             int bitIdxOffset = (int)cmd % sizeOfLong;
@@ -440,6 +445,11 @@ public static ReadOnlySpan<RespCommand> ExpandForACLs(this RespCommand cmd)
 
         internal const RespCommand LastDataCommand = RespCommand.EVALSHA;
 
+        /// <summary>
+        /// Last valid command (i.e. RespCommand with the largest value excluding INVALID).
+        /// </summary>
+        public static RespCommand LastValidCommand = Enum.GetValues<RespCommand>().Where(cmd => cmd != RespCommand.INVALID).Max();
+
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool IsReadOnly(this RespCommand cmd)
             => cmd <= LastReadCommand;

diff --git a/test/Garnet.test/RespCommandTests.cs b/test/Garnet.test/RespCommandTests.cs
@@ -383,7 +383,7 @@ public void AofIndependentCommandsTest()
                 RespCommand.MULTI,
             ];
 
-            foreach (RespCommand cmd in Enum.GetValues(typeof(RespCommand)))
+            foreach (var cmd in Enum.GetValues<RespCommand>().Where(cmd => cmd != RespCommand.INVALID))
             {
                 var expectedAofIndependence = Array.IndexOf(aofIndpendentCmds, cmd) != -1;
                 ClassicAssert.AreEqual(expectedAofIndependence, cmd.IsAofIndependent());