sysvars: make macros type-safe for the common case of var constant

If the variable is constant, we can assert type and validity at
runtime. For the important subcase of getvar(), we can completely
avoid calling the global function.

esp32/.gitignore

@@ -10,5 +10,5 @@ cache/
 *.elf
 *.map
 www/version
-max80/sysvars.h
-max80/sysvars.c
+*_gen.h
+*_gen.c

esp32/Makefile

@@ -6,7 +6,7 @@ PERL	      = perl
 
 SKETCH	      = max80
 TARGET	      = output/$(SKETCH).ino.bin
-GENFILES      = www.zip $(SKETCH)/sysvars.c $(SKETCH)/sysvars.h
+GENFILES      = www.zip $(SKETCH)/sysvars_gen.c $(SKETCH)/sysvars_gen.h
 WWW	      = www
 PORT	     ?= /dev/ttyACM0
 
@@ -36,8 +36,8 @@ $(TARGET): $(shell find $(SKETCH) -type f) $(GENFILES)
 	cd $(SKETCH) && \
 		$(ARDUINO_CLI) compile $(ARDUINO_OPTS)
 
-$(SKETCH)/%.c $(SKETCH)/%.h: %.vars sysvars.pl
-	$(PERL) sysvars.pl $< $(SKETCH)/$*.h $(SKETCH)/$*.c
+$(SKETCH)/%_gen.c $(SKETCH)/%_gen.h: %.vars sysvars.pl
+	$(PERL) sysvars.pl $< $(SKETCH)/$*_gen.h $(SKETCH)/$*_gen.c
 
 .PHONY: zip
 zip: zipexclude

esp32/max80/sysvar_common.h

@@ -1,89 +0,0 @@
-#ifndef SYSVAR_COMMON_H
-#define SYSVAR_COMMON_H
-
-#include <stddef.h>
-#include <stdbool.h>
-#include <inttypes.h>
-
-#ifndef extern_c
-# ifdef __cplusplus
-#  define extern_c extern "C"
-# else
-#  define extern_c extern
-# endif
-#endif
-
-typedef struct sysvar_namespace {
-    const char *name;
-    size_t first, count;
-} sysvar_ns_t;
-
-typedef union sysvar_value {
-    bool v_bool;
-    long int v_int;
-    unsigned long int v_uint;
-    const char *v_str;
-    const char *v_tz;		/* Timezone */
-    uint32_t v_ip;		/* IPv4 address */
-    const uint8_t *v_mac;	/* MAC address */
-    void *v_ptr;
-} sysvar_t;
-
-struct sysvar_ops {
-    bool (*set)(sysvar_t *, sysvar_t);
-    /* bool (*unset)(sysvar_t *); - not used */
-    const char * (*tostr)(sysvar_t, char *);
-    bool (*fromstr)(sysvar_t *, const char *);
-    void (*update)(sysvar_t, bool); /* Called after set or fromstr; bool = isset */
-    size_t buflen;		/* Minimal buffer size for string if needed */
-    bool is_ptr;		/* Type is an allocated pointer */
-};
-
-typedef const struct sysvar_ops *sysvar_type_t;
-
-#define SYSVAR_TYPE(x) (&sysvar_ ## x ## _ops)
-
-/* Buffer size needed to represent some data types */
-#define BOOL_BUFLEN 2
-#define INT_BUFLEN  (3*sizeof(unsigned int)+2)
-#define UINT_BUFLEN INT_BUFLEN
-#define IP_BUFLEN   (4*4)
-#define MAC_BUFLEN  (3*6)
-
-#define SYSVAR_BUFLEN	32	/* Conservative minimum */
-
-extern_c sysvar_t getvar(size_t var);
-extern_c bool setvar(size_t var, sysvar_t val);
-extern_c bool unsetvar(size_t var);
-extern_c const char *getvar_tostr(size_t var);
-extern_c const char *getvar_tostr_r(size_t var, char *buf);
-extern_c bool setvar_fromstr(size_t var, const char *str);
-extern_c void sysvar_reset(void);
-extern_c size_t sysvar_find(size_t ns, const char *name);
-
-extern_c const struct sysvar_ops sysvar_nulltype_ops;
-
-/* Type-specific definitions/getters/setters */
-
-#define SYSVAR_MKTYPE(t,c_type)					\
-    static inline c_type getvar_ ## t (size_t var)		\
-    {								\
-	return getvar(var).v_ ## t ;				\
-    }								\
-    static inline bool setvar_ ## t (size_t var, c_type v)	\
-    {								\
-	sysvar_t vv;						\
-	vv.v_ ## t = v;						\
-	return setvar(var, vv);					\
-    }								\
-    extern_c const struct sysvar_ops sysvar_ ## t ## _ops
-
-SYSVAR_MKTYPE(bool, bool);
-SYSVAR_MKTYPE(int, long int);
-SYSVAR_MKTYPE(uint, unsigned long int);
-SYSVAR_MKTYPE(str, const char *);
-SYSVAR_MKTYPE(tz, const char *);
-SYSVAR_MKTYPE(ip, uint32_t);
-SYSVAR_MKTYPE(mac, const uint8_t *);
-
-#endif /* SYSVAR_COMMON_H */

esp32/max80/sysvar.c → esp32/max80/sysvars.c

@@ -5,25 +5,6 @@
 #include <stdio.h>
 #include <time.h>
 
-static const char *sysvar_nulltype_tostr(sysvar_t from, char *buf)
-{
-    (void)from;
-    (void)buf;
-    return NULL;
-}
-
-static bool sysvar_nulltype_fromstr(sysvar_t *to, const char *from)
-{
-    (void)to;
-    (void)from;
-    return false;
-}
-
-const struct sysvar_ops sysvar_nulltype_ops = {
-    .tostr = sysvar_nulltype_tostr,
-    .fromstr = sysvar_nulltype_fromstr
-};
-
 static const char *sysvar_bool_tostr(sysvar_t from, char *buf)
 {
     buf[0] = '0' + from.v_bool;
@@ -266,7 +247,7 @@ sysvar_t getvar(size_t var)
 
 static bool do_setvar(size_t var, sysvar_t val, bool is_set)
 {
-    const struct sysvar_ops *type = sysvar_type[var];
+    const struct sysvar_ops *type = sysvar_types[var];
     sysvar_t *to = &sysvar_val[var];
     void *free_ptr = NULL;
 
@@ -319,10 +300,10 @@ const char *getvar_tostr(size_t var)
 
 const char *getvar_tostr_r(size_t var, char *buf)
 {
-    if (var >= (size_t)sysvar_count || !sysvar_isset[var])
+    if (var >= (size_t)sysvar_count)
 	return NULL;
 
-    const struct sysvar_ops *type = sysvar_type[var];
+    const struct sysvar_ops *type = sysvar_types[var];
 
     /* A tostr method is required */
     return type->tostr(sysvar_val[var], buf);
@@ -336,7 +317,7 @@ bool setvar_fromstr(size_t var, const char *str)
     if (!str)
 	return unsetvar(var);
 
-    const struct sysvar_ops *type = sysvar_type[var];
+    const struct sysvar_ops *type = sysvar_types[var];
     sysvar_t *to = &sysvar_val[var];
     void *free_ptr = NULL;
 

esp32/max80/sysvars.h

@@ -0,0 +1,118 @@
+#ifndef SYSVARS_H
+#define SYSVARS_H
+
+#include <stddef.h>
+#include <stdbool.h>
+#include <inttypes.h>
+
+#ifndef extern_c
+# ifdef __cplusplus
+#  define extern_c extern "C"
+# else
+#  define extern_c extern
+# endif
+#endif
+
+typedef union sysvar_value {
+    bool v_bool;
+    long int v_int;
+    unsigned long int v_uint;
+    const char *v_str;
+    const char *v_tz;		/* Timezone */
+    uint32_t v_ip;		/* IPv4 address */
+    const uint8_t *v_mac;	/* MAC address */
+    void *v_ptr;
+} sysvar_t;
+
+struct sysvar_ops {
+    bool (*set)(sysvar_t *, sysvar_t);
+    /* bool (*unset)(sysvar_t *); - not used */
+    const char * (*tostr)(sysvar_t, char *);
+    bool (*fromstr)(sysvar_t *, const char *);
+    void (*update)(sysvar_t, bool); /* Called after set or fromstr; bool = isset */
+    size_t buflen;		/* Minimal buffer size for string if needed */
+    bool is_ptr;		/* Type is an allocated pointer */
+};
+
+typedef const struct sysvar_ops *sysvar_type_t;
+
+extern_c const struct sysvar_ops sysvar_bool_ops;
+extern_c const struct sysvar_ops sysvar_int_ops;
+extern_c const struct sysvar_ops sysvar_uint_ops;
+extern_c const struct sysvar_ops sysvar_str_ops;
+extern_c const struct sysvar_ops sysvar_tz_ops;
+extern_c const struct sysvar_ops sysvar_ip_ops;
+extern_c const struct sysvar_ops sysvar_mac_ops;
+
+#define SYSVAR_NULLTYPE NULL
+#define SYSVAR_TYPE(x) (&sysvar ## x ## _ops)
+
+#include "sysvars_gen.h"
+
+typedef struct sysvar_namespace {
+    const char *name;
+    enum sysvars_enum first;
+    size_t count;
+} sysvar_ns_t;
+
+extern_c const sysvar_ns_t sysvar_ns[sysvar_nscount];
+extern_c enum sysvars_enum sysvar_changed;
+
+/* Buffer size needed to represent some data types */
+#define BOOL_BUFLEN 2
+#define INT_BUFLEN  (3*sizeof(unsigned int)+2)
+#define UINT_BUFLEN INT_BUFLEN
+#define IP_BUFLEN   (4*4)
+#define MAC_BUFLEN  (3*6)
+
+#define SYSVAR_BUFLEN	32	/* Conservative minimum */
+
+extern_c sysvar_t getvar(size_t var);
+extern_c bool setvar(size_t var, sysvar_t val);
+extern_c bool unsetvar(size_t var);
+extern_c const char *getvar_tostr(size_t var);
+extern_c const char *getvar_tostr_r(size_t var, char *buf);
+extern_c bool setvar_fromstr(size_t var, const char *str);
+extern_c void sysvar_reset(void);
+extern_c size_t sysvar_find(size_t ns, const char *name);
+
+/* Type-specific definitions/getters/setters */
+/* Note that t contains a leading underscore to avoid bool/_Bool issues */
+
+#define const_assert(cond, str)					     \
+    do {							     \
+	extern void fail(void) __attribute__((error(str)));	     \
+	if (__builtin_constant_p(cond) && !(cond))		     \
+	    fail();						     \
+    } while (0)
+
+#define TRY_ASSERT_TYPE(var,t)						\
+    const_assert(sysvar_type(var) == SYSVAR_TYPE(t),			\
+	"invalid type for sysvar " #var)
+
+#define SYSVAR_MKTYPE(t,c_type)						\
+    static inline c_type getvar ## t (size_t var)			\
+    {									\
+	TRY_ASSERT_TYPE(var,t);						\
+	/* If var is constant and >= sysvar_count, TRY_ASSERT_TYPE() fails */ \
+	if (__builtin_constant_p(var < (size_t)sysvar_count))		\
+	    return sysvar_val[var].v ## t;				\
+	return getvar(var).v ## t ;					\
+    }									\
+    static inline bool setvar ## t (size_t var, c_type v)		\
+    {									\
+	sysvar_t vv;							\
+	TRY_ASSERT_TYPE(var,t);						\
+	vv.v ## t = v;							\
+	return setvar(var, vv);						\
+    }
+
+SYSVAR_MKTYPE(_bool, bool);
+SYSVAR_MKTYPE(_int, long int);
+SYSVAR_MKTYPE(_uint, unsigned long int);
+SYSVAR_MKTYPE(_str, const char *);
+SYSVAR_MKTYPE(_tz, const char *);
+SYSVAR_MKTYPE(_ip, uint32_t);
+SYSVAR_MKTYPE(_mac, const uint8_t *);
+
+#endif /* SYSVARS_H */

esp32/sysvars.pl

@@ -60,7 +60,7 @@ my %nsfirst;
 my %nscount;
 my @varname = (undef);
 my @varenum = ('sysvar_null');
-my @vartype = ("\t[sysvar_null] = SYSVAR_TYPE(nulltype),\n");
+my %vartype = ('sysvar_null' => 'SYSVAR_NULLTYPE');
 my @vardef;
 
 my $cnt = 1;
@@ -73,7 +73,7 @@ foreach my $ns (@nslist) {
 	my $ename = c_name($ns.'.'.$var);
 	push(@varenum, $ename);
 
-	push(@vartype, "\t[$ename] = SYSVAR_TYPE($type),\n");
+	$vartype{$ename} = "SYSVAR_TYPE(_$type)";
 	if (defined($defval)) {
 	    # Add mangling here if needed
 	    push(@vardef, "\t[$ename] = { .v_$type = $defval },\n");
@@ -88,8 +88,6 @@ my $htag = c_name(uc(basename($hfile)));
 print $h "#ifndef $htag\n";
 print $h "#define $htag\n\n";
 
-print $h "#include \"sysvar_common.h\"\n\n";
-
 print $h "enum sysvar_ns_enum {\n";
 print $h map { "\tsysvar_ns_$_,\n" } @nslist;
 print $h "\tsysvar_nscount\n};\n\n";
@@ -102,20 +100,39 @@ push(@varenum, 'sysvar_count');
 
 print $h "\n";
 
-printf $h "extern_c const sysvar_ns_t sysvar_ns[%d];\n", scalar(@nslist);
-print $h "extern_c const char * const sysvar_name[$cnt];\n";
-print $h "extern_c const sysvar_type_t sysvar_type[$cnt];\n";
-print $h "extern_c const sysvar_t sysvar_defval[$cnt];\n";
-print $h "extern_c sysvar_t sysvar_val[$cnt];\n";
-print $h "extern_c bool sysvar_isset[$cnt];\n";
-print $h "extern_c enum sysvars_enum sysvar_changed;\n";
+# Sometimes it is convenient to have this as a preprocessor symbol
+print $h "#define SYSVAR_COUNT $cnt\n\n";
+
+print $h "extern_c const char * const sysvar_name[sysvar_count];\n";
+print $h "extern_c const sysvar_type_t sysvar_types[sysvar_count];\n";
+print $h "extern_c const sysvar_t sysvar_defval[sysvar_count];\n";
+print $h "extern_c sysvar_t sysvar_val[sysvar_count];\n";
+print $h "extern_c bool sysvar_isset[sysvar_count];\n\n";
+
+print $h "static inline sysvar_type_t sysvar_type(size_t var)\n";
+print $h "{\n";
+print $h "\tif (!__builtin_constant_p(var)) {\n";
+print $h "\t\tif(var >= (size_t)sysvar_count)\n";
+print $h "\t\t\treturn SYSVAR_NULLTYPE;\n";
+print $h "\t\telse\n";
+print $h "\t\t\treturn sysvar_types[var];\n";
+print $h "\t}\n\n";
+print $h "\tswitch(var) {\n";
+foreach my $v (@varenum) {
+    next unless (defined($vartype{$v}));
+    print $h "\t\tcase ", $v, ":\n";
+    print $h "\t\t\treturn ", $vartype{$v}, ";\n";
+}
+print $h "\t\tdefault:\n";
+print $h "\t\t\treturn SYSVAR_NULLTYPE;\n";
+print $h "\t};\n";
+print $h "}\n\n";
 
-print $h "\n#endif /* $htag */\n";
+print $h "#endif /* $htag */\n";
 close($h);
 
 open(my $c, '>', $cfile) or die "$0:$cfile: $!\n";
-print $c "#include <stddef.h>\n";
-print $c "#include \"", basename($hfile), "\"\n\n";
+print $c "#include \"sysvars.h\"\n\n";
 
 printf $c "const sysvar_ns_t sysvar_ns[%d] = {\n", scalar(@nslist);
 foreach my $ns (@nslist) {
@@ -134,8 +151,11 @@ foreach my $ns (@nslist) {
 }
 print $c "};\n";
 
-print $c "const sysvar_type_t sysvar_type[$cnt] = {\n";
-print $c @vartype;
+print $c "const sysvar_type_t sysvar_types[$cnt] = {\n";
+foreach my $v (@varenum) {
+    next unless (defined($vartype{$v}));
+    print $c "\t[$v] = ", $vartype{$v}, ",\n";
+}
 print $c "};\n";
 
 print $c "const sysvar_t sysvar_defval[$cnt] = {\n";