[Github-comments] [geany/geany] Rewrite HL_N_ENTRIES macro to avoid a GCC8 false positive warning (#2398)

[Colomban Wendling]

> @b4n for the places this is used, is there a (compile time testable) value of array element you could use as a non-entry so its always arrays and so doesn't mix arrays and pointers, then the GCC warning will catch if anyone does call it with a pointer?

None that I can think of; the non-array values are `EMPTY_KEYWORDS` and `EMPTY_PROPERTIES`.  If ISO C allowed it, the simple fix would be to declare those as empty arrays, and it would naturally work out of the box.  But without this, the best other solution I can think of is this:
```diff
diff --git a/src/highlighting.c b/src/highlighting.c
index 938e5432c..82013aa7c 100644
--- a/src/highlighting.c
+++ b/src/highlighting.c
@@ -961,9 +961,9 @@ static guint get_lexer_filetype(GeanyFiletype *ft)
 	case (GEANY_FILETYPES_##LANG_NAME): \
 		styleset_init_from_mapping(filetype_idx, config, configh, \
 				highlighting_styles_##LANG_NAME, \
-				HL_N_ENTRIES(highlighting_styles_##LANG_NAME), \
+				HL_N_STYLES(highlighting_styles_##LANG_NAME), \
 				highlighting_keywords_##LANG_NAME, \
-				HL_N_ENTRIES(highlighting_keywords_##LANG_NAME)); \
+				HL_N_KEYWORDS(highlighting_keywords_##LANG_NAME)); \
 		break
 
 /* Called by filetypes_load_config(). */
@@ -1068,11 +1068,11 @@ void highlighting_init_styles(guint filetype_idx, GKeyFile *config, GKeyFile *co
 	case (GEANY_FILETYPES_##LANG_NAME): \
 		styleset_from_mapping(sci, ft->id, highlighting_lexer_##LANG_NAME, \
 				highlighting_styles_##LANG_NAME, \
-				HL_N_ENTRIES(highlighting_styles_##LANG_NAME), \
+				HL_N_STYLES(highlighting_styles_##LANG_NAME), \
 				highlighting_keywords_##LANG_NAME, \
-				HL_N_ENTRIES(highlighting_keywords_##LANG_NAME), \
+				HL_N_KEYWORDS(highlighting_keywords_##LANG_NAME), \
 				highlighting_properties_##LANG_NAME, \
-				HL_N_ENTRIES(highlighting_properties_##LANG_NAME)); \
+				HL_N_PROPERTIES(highlighting_properties_##LANG_NAME)); \
 		break
 
 /** Sets up highlighting and other visual settings.
diff --git a/src/highlightingmappings.h b/src/highlightingmappings.h
index d62b7e24b..36fe2d260 100644
--- a/src/highlightingmappings.h
+++ b/src/highlightingmappings.h
@@ -65,11 +65,14 @@ typedef struct
 } HLProperty;
 
 
-#define EMPTY_KEYWORDS		((HLKeyword *) NULL)
-#define EMPTY_PROPERTIES	((HLProperty *) NULL)
-
-/* like G_N_ELEMENTS() but supports @array being NULL (for empty entries) */
-#define HL_N_ENTRIES(array) ((array != NULL) ? G_N_ELEMENTS(array) : 0)
+/* ISO C doesn't allow zero-sized arrays, so we special-case those below */
+static const HLKeyword EMPTY_KEYWORDS[] = {{0}};
+static const HLProperty EMPTY_PROPERTIES[] = {{0}};
+
+/* like G_N_ELEMENTS() but supports special values for empty entries */
+#define HL_N_STYLES(ss) G_N_ELEMENTS(ss)
+#define HL_N_KEYWORDS(ks) ((ks) == EMPTY_KEYWORDS ? 0 : G_N_ELEMENTS(ks))
+#define HL_N_PROPERTIES(ps) ((ps) == EMPTY_PROPERTIES ? 0 : G_N_ELEMENTS(ps))
 
 
 /* Abaqus */
```
but I'm not really convinced it's actually better.  I'm not against it, it's not really bad but it's kind of sad it wastes `sizeof(HLKeyword) + sizeof(HLProperty)` (40 bytes here) and introduces 2 new specific macros.

As for getting called with a pointer it's a theoretically good point, but given it's only used on highlightingmappings entries, it should be fairly safe.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2398#issuecomment-554753212
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20191117/8dcb3757/attachment.html>