Cleanup (#826)

* Update meson.build
- add logic to loop over more lua names (in the future more names might be discovered)
- disable warnings and errors on dependencies

* adding missing includes and checks, correct data types, pointer mess […]
- various functions from string.h were used but never defined
- logic was done across multiple different data types with different signedness, got all of them up to snuff
- give 0 sized array size of 1 (array of size 0 is illegal, but rewriting the code is out of the scope of this commit)
- add preprocessor that marks possibly unused argument as such (does not mean they will get optimized out or anything)
- correctly initialize structs with all data needed

All these were found by generating the project using `meson -Dwarning_level=3 -Dwerror=true`

* remove undefined behavior, correct data types

* Comment manual bit manipulation to be investigated

* check for more edge cases, replace multiple cleanups with goto

* remove system specific includes

src/api/dirmonitor.c

@@ -1,12 +1,7 @@
 #include "api.h"
 #include <stdlib.h>
-#ifdef _WIN32
+#ifdef DIRMONITOR_WIN32
   #include <windows.h>
-#elif __linux__
-  #include <sys/inotify.h>
-  #include <limits.h>
-#else
-  #include <sys/event.h>
 #endif
 #include <unistd.h>
 #include <errno.h>

src/api/process.c

@@ -5,6 +5,7 @@
 #include <stdint.h>
 #include <stdlib.h>
 #include <SDL.h>
+#include <assert.h>
 
 #if _WIN32
   // https://stackoverflow.com/questions/60645/overlapped-i-o-on-anonymous-pipe
@@ -21,19 +22,23 @@
 
 #define READ_BUF_SIZE 2048
 
+#if _WIN32
+typedef HANDLE process_handle;
+#else
+typedef int process_handle;
+#endif
+
 typedef struct {
   bool running;
   int returncode, deadline;
   long pid;
   #if _WIN32
     PROCESS_INFORMATION process_information;
-    HANDLE child_pipes[3][2];
     OVERLAPPED overlapped[2];
     bool reading[2];
     char buffer[2][READ_BUF_SIZE];
-  #else
-    int child_pipes[3][2];
   #endif
+    process_handle child_pipes[3][2];
 } process_t;
 
 typedef enum {
@@ -91,7 +96,7 @@ static bool poll_process(process_t* proc, int timeout) {
     #endif
     if (timeout)
       SDL_Delay(5);
-  } while (timeout == WAIT_INFINITE || SDL_GetTicks() - ticks < timeout);
+  } while (timeout == WAIT_INFINITE || (int)SDL_GetTicks() - ticks < timeout);
 
   return proc->running;
 }
@@ -117,8 +122,9 @@ static bool signal_process(process_t* proc, signal_e sig) {
 }
 
 static int process_start(lua_State* L) {
+  int retval = 1;
   size_t env_len = 0, key_len, val_len;
-  const char *cmd[256], *env_names[256] = { NULL }, *env_values[256] = { NULL }, *cwd = NULL;
+  const char *cmd[256] = { NULL }, *env_names[256] = { NULL }, *env_values[256] = { NULL }, *cwd = NULL;
   bool detach = false;
   int deadline = 10, new_fds[3] = { STDIN_FD, STDOUT_FD, STDERR_FD };
   luaL_checktype(L, 1, LUA_TTABLE);
@@ -134,7 +140,11 @@ static int process_start(lua_State* L) {
     lua_rawget(L, 1);
     cmd[i-1] = luaL_checkstring(L, -1);
   }
-  cmd[cmd_len] = NULL;
+
+  // this should never trip
+  // but if it does we are in deep trouble
+  assert(cmd[0]);
+
   if (arg_len > 1) {
     lua_getfield(L, 2, "env");
     if (!lua_isnil(L, -1)) {
@@ -158,8 +168,14 @@ static int process_start(lua_State* L) {
     lua_getfield(L, 2, "stdout");  new_fds[STDOUT_FD] = luaL_optnumber(L, -1, STDOUT_FD);
     lua_getfield(L, 2, "stderr");  new_fds[STDERR_FD] = luaL_optnumber(L, -1, STDERR_FD);
     for (int stream = STDIN_FD; stream <= STDERR_FD; ++stream) {
-      if (new_fds[stream] > STDERR_FD || new_fds[stream] < REDIRECT_PARENT)
-        return luaL_error(L, "redirect to handles, FILE* and paths are not supported");
+      if (new_fds[stream] > STDERR_FD || new_fds[stream] < REDIRECT_PARENT) {
+        for (size_t i = 0; i < env_len; ++i) {
+          free((char*)env_names[i]);
+          free((char*)env_values[i]);
+        }
+        retval = luaL_error(L, "redirect to handles, FILE* and paths are not supported");
+        goto cleanup;
+      }
     }
   }
   
@@ -188,16 +204,21 @@ static int process_start(lua_State* L) {
             sprintf(pipeNameBuffer, "\\\\.\\Pipe\\RemoteExeAnon.%08lx.%08lx", GetCurrentProcessId(), InterlockedIncrement(&PipeSerialNumber));
             self->child_pipes[i][0] = CreateNamedPipeA(pipeNameBuffer, PIPE_ACCESS_INBOUND | FILE_FLAG_OVERLAPPED,
               PIPE_TYPE_BYTE | PIPE_WAIT, 1, READ_BUF_SIZE, READ_BUF_SIZE, 0, NULL);
-            if (self->child_pipes[i][0] == INVALID_HANDLE_VALUE)
-              return luaL_error(L, "Error creating read pipe: %d.", GetLastError());
+            if (self->child_pipes[i][0] == INVALID_HANDLE_VALUE) {
+              retval = luaL_error(L, "Error creating read pipe: %d.", GetLastError());
+              goto cleanup;
+            }
             self->child_pipes[i][1] = CreateFileA(pipeNameBuffer, GENERIC_WRITE, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
             if (self->child_pipes[i][1] == INVALID_HANDLE_VALUE) {
               CloseHandle(self->child_pipes[i][0]);
-              return luaL_error(L, "Error creating write pipe: %d.", GetLastError());
+              retval = luaL_error(L, "Error creating write pipe: %d.", GetLastError());
+              goto cleanup;
             }
             if (!SetHandleInformation(self->child_pipes[i][i == STDIN_FD ? 1 : 0], HANDLE_FLAG_INHERIT, 0) ||
-                !SetHandleInformation(self->child_pipes[i][i == STDIN_FD ? 0 : 1], HANDLE_FLAG_INHERIT, 1))
-                  return luaL_error(L, "Error inheriting pipes: %d.", GetLastError());
+                !SetHandleInformation(self->child_pipes[i][i == STDIN_FD ? 0 : 1], HANDLE_FLAG_INHERIT, 1)) {
+                  retval = luaL_error(L, "Error inheriting pipes: %d.", GetLastError());
+                  goto cleanup;
+            }
           }
         } break;
       }
@@ -237,24 +258,25 @@ static int process_start(lua_State* L) {
     environmentBlock[offset++] = 0;
     if (env_len > 0)
       MultiByteToWideChar(CP_UTF8, MB_PRECOMPOSED, environmentBlock, offset, (LPWSTR)wideEnvironmentBlock, sizeof(wideEnvironmentBlock));
-    if (!CreateProcess(NULL, commandLine, NULL, NULL, true, (detach ? DETACHED_PROCESS : CREATE_NO_WINDOW) | CREATE_UNICODE_ENVIRONMENT, env_len > 0 ? wideEnvironmentBlock : NULL, cwd, &siStartInfo, &self->process_information))
-      return luaL_error(L, "Error creating a process: %d.", GetLastError());
+    if (!CreateProcess(NULL, commandLine, NULL, NULL, true, (detach ? DETACHED_PROCESS : CREATE_NO_WINDOW) | CREATE_UNICODE_ENVIRONMENT, env_len > 0 ? wideEnvironmentBlock : NULL, cwd, &siStartInfo, &self->process_information)) {
+      retval = luaL_error(L, "Error creating a process: %d.", GetLastError());
+      goto cleanup;
+    }
     self->pid = (long)self->process_information.dwProcessId;
     if (detach) 
       CloseHandle(self->process_information.hProcess);
     CloseHandle(self->process_information.hThread);
   #else
     for (int i = 0; i < 3; ++i) { // Make only the parents fd's non-blocking. Children should block.
-      if (pipe(self->child_pipes[i]) || fcntl(self->child_pipes[i][i == STDIN_FD ? 1 : 0], F_SETFL, O_NONBLOCK) == -1)
-        return luaL_error(L, "Error creating pipes: %s", strerror(errno));
+      if (pipe(self->child_pipes[i]) || fcntl(self->child_pipes[i][i == STDIN_FD ? 1 : 0], F_SETFL, O_NONBLOCK) == -1) {
+        retval = luaL_error(L, "Error creating pipes: %s", strerror(errno));
+        goto cleanup;
+      }
     }
     self->pid = (long)fork();
     if (self->pid < 0) {
-      for (int i = 0; i < 3; ++i) {
-        close(self->child_pipes[i][0]);
-        close(self->child_pipes[i][1]);
-      }
-      return luaL_error(L, "Error running fork: %s.", strerror(errno));
+      retval = luaL_error(L, "Error running fork: %s.", strerror(errno));
+      goto cleanup;
     } else if (!self->pid) {
       setpgrp();
       for (int stream = 0; stream < 3; ++stream) {
@@ -265,23 +287,28 @@ static int process_start(lua_State* L) {
           dup2(self->child_pipes[new_fds[stream]][new_fds[stream] == STDIN_FD ? 0 : 1], stream);
         close(self->child_pipes[stream][stream == STDIN_FD ? 1 : 0]);
       }
-      int set;
+      size_t set;
       for (set = 0; set < env_len && setenv(env_names[set], env_values[set], 1) == 0; ++set);
       if (set == env_len && (!detach || setsid() != -1) && (!cwd || chdir(cwd) != -1))
-        execvp((const char*)cmd[0], (char* const*)cmd);
+        execvp(cmd[0], (char** const)cmd);
       const char* msg = strerror(errno);
-      int result = write(STDERR_FD, msg, strlen(msg)+1);
+      size_t result = write(STDERR_FD, msg, strlen(msg)+1);
       _exit(result == strlen(msg)+1 ? -1 : -2);
     }
   #endif
+  cleanup:
   for (size_t i = 0; i < env_len; ++i) {
     free((char*)env_names[i]);
     free((char*)env_values[i]);
   }
-  for (int stream = 0; stream < 3; ++stream)
-    close_fd(&self->child_pipes[stream][stream == STDIN_FD ? 0 : 1]);
+  for (int stream = 0; stream < 3; ++stream) {
+    process_handle* pipe = &self->child_pipes[stream][stream == STDIN_FD ? 0 : 1];
+    if (*pipe) {
+      close_fd(pipe);
+    }
+  }
   self->running = true;
-  return 1;
+  return retval;
 }
 
 static int g_read(lua_State* L, int stream, unsigned long read_size) {

src/api/renderer.c

@@ -1,3 +1,4 @@
+#include <string.h>
 #include "api.h"
 #include "../renderer.h"
 #include "../rencache.h"
@@ -166,14 +167,14 @@ static int f_get_size(lua_State *L) {
 }
 
 
-static int f_begin_frame(lua_State *L) {
-  rencache_begin_frame(L);
+static int f_begin_frame(UNUSED lua_State *L) {
+  rencache_begin_frame();
   return 0;
 }
 
 
-static int f_end_frame(lua_State *L) {
-  rencache_end_frame(L);
+static int f_end_frame(UNUSED lua_State *L) {
+  rencache_end_frame();
   return 0;
 }
 
@@ -214,7 +215,7 @@ static int f_draw_text(lua_State *L) {
   float x = luaL_checknumber(L, 3);
   int y = luaL_checknumber(L, 4);
   RenColor color = checkcolor(L, 5, 255);
-  x = rencache_draw_text(L, fonts, text, x, y, color);
+  x = rencache_draw_text(fonts, text, x, y, color);
   lua_pushnumber(L, x);
   return 1;
 }

src/api/system.c

@@ -45,7 +45,7 @@ struct HitTestInfo {
 };
 typedef struct HitTestInfo HitTestInfo;
 
-static HitTestInfo window_hit_info[1] = {{0, 0}};
+static HitTestInfo window_hit_info[1] = {{0, 0, 0}};
 
 #define RESIZE_FROM_TOP 0
 #define RESIZE_FROM_RIGHT 0
@@ -705,13 +705,15 @@ static int f_set_window_opacity(lua_State *L) {
   return 1;
 }
 
+typedef void (*fptr)(void);
+
 typedef struct lua_function_node {
   const char *symbol;
-  void *address;
+  fptr address;
 } lua_function_node;
 
-#define P(FUNC) { "lua_" #FUNC, (void*)(lua_##FUNC) }
-#define U(FUNC) { "luaL_" #FUNC, (void*)(luaL_##FUNC) }
+#define P(FUNC) { "lua_" #FUNC, (fptr)(lua_##FUNC) }
+#define U(FUNC) { "luaL_" #FUNC, (fptr)(luaL_##FUNC) }
 static void* api_require(const char* symbol) {
   static lua_function_node nodes[] = {
     P(atpanic), P(checkstack),
@@ -749,9 +751,9 @@ static void* api_require(const char* symbol) {
     #endif
 
   };
-  for (int i = 0; i < sizeof(nodes) / sizeof(lua_function_node); ++i) {
+  for (size_t i = 0; i < sizeof(nodes) / sizeof(lua_function_node); ++i) {
     if (strcmp(nodes[i].symbol, symbol) == 0)
-      return nodes[i].address;
+      return *(void**)(&nodes[i].address);
   }
   return NULL;
 }
@@ -775,10 +777,12 @@ static int f_load_native_plugin(lua_State *L) {
   const char *basename = strrchr(name, '.');
   basename = !basename ? name : basename + 1;
   snprintf(entrypoint_name, sizeof(entrypoint_name), "luaopen_lite_xl_%s", basename);
-  int (*ext_entrypoint) (lua_State *L, void*) = SDL_LoadFunction(library, entrypoint_name);
+  int (*ext_entrypoint) (lua_State *L, void* (*)(const char*));
+  *(void**)(&ext_entrypoint) = SDL_LoadFunction(library, entrypoint_name);
   if (!ext_entrypoint) {
     snprintf(entrypoint_name, sizeof(entrypoint_name), "luaopen_%s", basename);
-    int (*entrypoint)(lua_State *L) = SDL_LoadFunction(library, entrypoint_name);
+    int (*entrypoint)(lua_State *L);
+    *(void**)(&entrypoint) = SDL_LoadFunction(library, entrypoint_name);
     if (!entrypoint)
       return luaL_error(L, "Unable to load %s: Can't find %s(lua_State *L, void *XL)", name, entrypoint_name);
     result = entrypoint(L);