src: using fixed-size buffers for arguments handling

Thanks to LHF for suggestion.

src/lua-hiredis.c

@@ -22,6 +22,8 @@ extern "C" {
 
 #define LUAHIREDIS_CONN_MT "lua-hiredis.connection"
 
+#define LUAHIREDIS_MAXARGS (256)
+
 #define LUAHIREDIS_NIL_KEY "NIL"
 static const int NIL_TOKEN = 1; /* TODO: Is this the best solution possible? */
 
@@ -97,36 +99,14 @@ static int push_error(lua_State * L, redisContext * pContext)
   return 3;
 }
 
-/* TODO: How to get rid of allocations here? */
-
-static void destroy_args(
-    lua_State * L,
-    int nargs,
-    const char *** argv,
-    size_t ** argvlen
-  )
-{
-  void * alloc_ud = NULL;
-  lua_Alloc alloc_fn = lua_getallocf(L, &alloc_ud);
-
-  alloc_fn(alloc_ud, (void *)*argvlen, nargs * sizeof(size_t), 0UL);
-  *argvlen = NULL;
-
-  alloc_fn(alloc_ud, (void *)*argv, nargs * sizeof(const char *), 0UL);
-  *argv = NULL;
-}
-
-static int create_args(
+static int load_args(
     lua_State * L,
     redisContext * pContext,
     int idx, /* index of first argument */
-    const char *** argv,
-    size_t ** argvlen
+    const char ** argv,
+    size_t * argvlen
   )
 {
-  void * alloc_ud = NULL;
-  lua_Alloc alloc_fn = lua_getallocf(L, &alloc_ud);
-
   int nargs = lua_gettop(L) - idx + 1;
   int i = 0;
 
@@ -135,23 +115,9 @@ static int create_args(
     return luaL_error(L, "missing command name");
   }
 
-  *argv = (const char **)alloc_fn(
-      alloc_ud, NULL, 0UL, nargs * sizeof(const char *)
-    );
-  if (*argv == NULL)
+  if (nargs > LUAHIREDIS_MAXARGS)
   {
-    return luaL_error(L, "command: can't allocate argv buffer");
-  }
-
-  *argvlen = (size_t *)alloc_fn(
-      alloc_ud, NULL, 0UL, nargs * sizeof(size_t)
-    );
-  if (*argvlen == NULL)
-  {
-    alloc_fn(alloc_ud, *argv, nargs * sizeof(const char *), 0UL);
-    *argv = NULL;
-
-    return luaL_error(L, "command: can't allocate argvlen buffer");
+    return luaL_error(L, "too many arguments");
   }
 
   for (i = 0; i < nargs; ++i)
@@ -161,13 +127,11 @@ static int create_args(
 
     if (str == NULL)
     {
-      destroy_args(L, nargs, argv, argvlen);
-
       return luaL_argerror(L, idx + i, "expected a string or number value");
     }
 
-    (*argv)[i] = str;
-    (*argvlen)[i] = len;
+    argv[i] = str;
+    argvlen[i] = len;
   }
 
   return nargs;
@@ -258,18 +222,16 @@ static int lconn_command(lua_State * L)
 {
   redisContext * pContext = check_connection(L, 1);
 
-  const char ** argv = NULL;
-  size_t * argvlen = NULL;
-  int nargs = create_args(L, pContext, 2, &argv, &argvlen);
+  const char * argv[LUAHIREDIS_MAXARGS];
+  size_t argvlen[LUAHIREDIS_MAXARGS];
+  int nargs = load_args(L, pContext, 2, argv, argvlen);
 
   int nret = 0;
   int i = 0;
 
   redisReply * pReply = redisCommandArgv(pContext, nargs, argv, argvlen);
   if (pReply == NULL)
   {
-    destroy_args(L, nargs, &argv, &argvlen);
-
     /* TODO: Shouldn't we clear the context error state after this? */
     return push_error(L, pContext);
   }
@@ -279,8 +241,6 @@ static int lconn_command(lua_State * L)
   freeReplyObject(pReply);
   pReply = NULL;
 
-  destroy_args(L, nargs, &argv, &argvlen);
-
   return nret;
 }
 

test/test.lua

@@ -4,7 +4,7 @@ pcall(require, 'luarocks.require')
 
 local hiredis = require 'hiredis'
 
-print(hiredis.connect("xxx", 1))
+assert(hiredis.connect("xxx", 1) == nil)
 local conn = assert(hiredis.connect("localhost", 6379))
 
 assert(conn:command("SET", "MYKEY", "MYVALUE"))
@@ -13,39 +13,57 @@ assert(assert(conn:command("GET", "MYKEY")) == "MYVALUE")
 local NIL = assert(conn:command("GET", "BADKEY"))
 assert(NIL == hiredis.NIL)
 
-print(conn:command("SET")) -- Not enough args
+assert(conn:command("SET") == nil) -- Not enough args
 
-print(assert(conn:command("MULTI")))
-print(assert(conn:command("SET", "MYKEY1", "MYVALUE1")))
+do
+  local a = { }
+  for i = 1, 512 do
+    a[#a + 1] = "SET"
+  end
+  -- Too many arguments
+  assert(pcall(conn.command, conn, unpack(a)) == false)
+end
+
+assert(assert(conn:command("MULTI")) == "OK")
+assert(assert(conn:command("SET", "MYKEY1", "MYVALUE1")) == "QUEUED")
 assert(assert(conn:command("GET", "MYKEY1")) == "QUEUED")
 local t = assert(conn:command("EXEC"))
 assert(t[1] == "OK")
 assert(t[2] == "MYVALUE1")
 
-print(assert(conn:command("MULTI")))
+assert(conn:command("MULTI"))
 assert(assert(conn:command("GET", "MYKEY1")) == "QUEUED")
-print(conn:command("SET")) -- Not enough args
+assert(conn:command("SET") == nil) -- Not enough args
 assert(assert(conn:command("GET", "MYKEY1")) == "QUEUED")
 local t = assert(conn:command("EXEC"))
 
 for i = 1, #t do
-  print(type(t[i]))
+  assert(t[i] == "MYVALUE1")
 end
-print("EXEC:", unpack(t))
 
-print("---")
+assert(conn:command("MULTI"))
 
-print(assert(conn:command("MULTI")))
--- Wrong type
+assert(assert(conn:command("SET", "MYKEY2", 1)) == "QUEUED")
+
+-- Wrong value type
 assert(assert(conn:command("SADD", "MYKEY1", "MYVAL")) == "QUEUED")
-assert(assert(conn:command("GET", "MYKEY1")) == "QUEUED")
+
+assert(assert(conn:command("INCR", "MYKEY2")) == "QUEUED")
 local t = assert(conn:command("EXEC"))
-for i = 1, #t do
-  print(type(t[i]))
+local r =
+{
+  "OK",
+  "Operation against a key holding the wrong kind of value",
+  2
+}
+assert(#t == #r)
+for i = 1, #r do
+  assert(t[i] == r[i])
 end
-print("EXEC:", unpack(t))
 
 conn:close()
 conn:close()
 
 error("TODO: Write better tests")
+
+print("OK")