The attached patch fixes these bugs in the colourful -dump code:

1. If neither CONFIG_88_COLORS nor CONFIG_256_COLORS is defined,
   then dump_to_file_256 is defined in dump.c but not used.
   If configure --enable-debug was used, then gcc warns about
   the unused function, and the warning stops the build.

2. The description of document.dump.color_mode ends with a
   newline, provoking a runtime warning from check_description
   in src/config/options.c.

3. options.inc has preprocessor directives inside macro arguments.
   That is not portable C.  xgettext (GNU gettext-tools) 0.14.3 is
   not smart enough to figure out the possible combinations, and
   copies an incorrect string to elinks.pot.

Some output resulting from these bugs:

make -C dump all
make[6]: Entering directory 
`/home/kalle/build/i686-pc-linux-gnu/elinks/src/viewer/dump'
gcc -DHAVE_CONFIG_H -I../../.. -I/home/kalle/src/elinks/src -O0 -ggdb -Wall 
-Wno-unused -Wall -Werror -fno-strict-aliasing -Wno-pointer-sign -o dump.o -c 
/home/kalle/src/elinks/src/viewer/dump/dump.c
cc1: warnings being treated as errors
/home/kalle/src/elinks/src/viewer/dump/dump.c:539: warning: ‘dump_to_file_256’ 
defined but not used
make[6]: *** [dump.o] Error 1
make[6]: Leaving directory 
`/home/kalle/build/i686-pc-linux-gnu/elinks/src/viewer/dump'

DEBUG MESSAGE at /home/kalle/src/elinks/src/config/options.c:120: bad char at 
end of description [Color mode for dumps:
-1 is standard dump mode
0 is mono mode
1 is 16 color mode
]

In my opinion, it's bad to make the meanings of possible values
of document.dump.color_mode vary depending on how ELinks was
configured.  It would be better to always have the same meanings,
and then document which of them the executable does not support
and what it will do instead.  However, I made no attempt to fix
that in the patch.

diff --git a/src/config/options.inc b/src/config/options.inc
index 2e79f8f..97b1c9b 100644
--- a/src/config/options.inc
+++ b/src/config/options.inc
@@ -632,29 +632,43 @@ static struct option_info config_options
 		N_("Codepage used in dump output. 'System' stands for\n"
 		"a codepage determined by a selected locale.")),
 
-	INIT_OPT_INT("document.dump", N_("Color mode"),
-		"color_mode", 0, -1,
+	/* The #if directives cannot be inside the argument list of
+	 * the INIT_OPT_INT macro; that wouldn't be standard C.
+	 * And they especially cannot be inside the argument list of N_;
+	 * xgettext (GNU gettext-tools) 0.14.3 wouldn't support that. */
 #if defined(CONFIG_88_COLORS) && defined(CONFIG_256_COLORS)
-3,
-#elif defined(CONFIG_88_COLORS) || defined(CONFIG_256_COLORS)
-2,
-#else
-1,
-#endif
--1,
+	INIT_OPT_INT("document.dump", N_("Color mode"),
+		"color_mode", 0, -1, 3, -1,
 		N_("Color mode for dumps:\n"
 		"-1 is standard dump mode\n"
 		"0 is mono mode\n"
 		"1 is 16 color mode\n"
-#if defined(CONFIG_88_COLORS) && defined(CONFIG_256_COLORS)
-		"2 is 88 color mode\n"
-		"3 is 256 color mode\n"
-#elif defined(CONFIG_88_COLORS)
 		"2 is 88 color mode\n"
-#elif defined(CONFIG_256_COLORS)
-		"2 is 256 color mode\n"
-#endif
-		)),
+		"3 is 256 color mode")),
+#elif defined(CONFIG_88_COLORS) /* && !defined(CONFIG_256_COLORS) */
+	INIT_OPT_INT("document.dump", N_("Color mode"),
+		"color_mode", 0, -1, 2, -1,
+		N_("Color mode for dumps:\n"
+		"-1 is standard dump mode\n"
+		"0 is mono mode\n"
+		"1 is 16 color mode\n"
+		"2 is 88 color mode")),
+#elif defined(CONFIG_256_COLORS) /* && !defined(CONFIG_88_COLORS) */
+	INIT_OPT_INT("document.dump", N_("Color mode"),
+		"color_mode", 0, -1, 2, -1,
+		N_("Color mode for dumps:\n"
+		"-1 is standard dump mode\n"
+		"0 is mono mode\n"
+		"1 is 16 color mode\n"
+		"2 is 256 color mode")),
+#else /* !defined(CONFIG_88_COLORS) && !defined(CONFIG_256_COLORS) */
+	INIT_OPT_INT("document.dump", N_("Color mode"),
+		"color_mode", 0, -1, 1, -1,
+		N_("Color mode for dumps:\n"
+		"-1 is standard dump mode\n"
+		"0 is mono mode\n"
+		"1 is 16 color mode")),
+#endif /* !defined(CONFIG_88_COLORS) && !defined(CONFIG_256_COLORS) */
 
 	INIT_OPT_STRING("document.dump", N_("Footer"),
 		"footer", 0, "",
diff --git a/src/viewer/dump/dump.c b/src/viewer/dump/dump.c
index 6179f6f..6b2a5e0 100644
--- a/src/viewer/dump/dump.c
+++ b/src/viewer/dump/dump.c
@@ -51,7 +51,9 @@ static struct download dump_download;
 static int dump_redir_count = 0;
 
 static int dump_to_file_16(struct document *document, int fd);
+#if defined(CONFIG_88_COLORS) || defined(CONFIG_256_COLORS)
 static int dump_to_file_256(struct document *document, int fd);
+#endif
 
 /* This dumps the given @cached's source onto @fd nothing more. It returns 0 if it
  * all went fine and 1 if something isn't quite right and we should terminate
@@ -521,6 +523,11 @@ fail:
 	return 0;
 }
 
+/* configure --enable-debug uses gcc -Wall -Werror, and -Wall includes
+ * -Wunused-function, so declaring or defining any unused function
+ * would break the build. */
+#if defined(CONFIG_88_COLORS) || defined(CONFIG_256_COLORS)
+
 static int
 write_color_256(unsigned char *str, unsigned char color, int fd, unsigned char *buf, int *bptr)
 {
@@ -648,6 +655,7 @@ fail:
 	return 0;
 }
 
+#endif /* defined(CONFIG_88_COLORS) || defined(CONFIG_256_COLORS) */
 
 int
 dump_to_file(struct document *document, int fd)

Attachment: pgpKhqTbTQy0e.pgp
Description: PGP signature

_______________________________________________
elinks-dev mailing list
[email protected]
http://linuxfromscratch.org/mailman/listinfo/elinks-dev

Reply via email to