[PATCH] no config file overwrite for dpkg

[Rob Landley]

On Sunday 21 February 2010 06:28:28 Denys Vlasenko wrote:
> On Friday 19 February 2010 15:32, Rob Landley wrote:
> > All modern compilers (and most obsolete ones) do simple dead code
> > elimination, where:
> >
> >    if (0) {
> >     code code code
> >   }
> >
> > Gets eliminated just like this would:
> >
> >   #if 0
> >     code code code
> >   #endif
> >
> > It also works the other way:
> >
> >   if (1) {
> >     code code code
> >   }
>
> In console-tools/loadfont.c, we now have this:
>
> #if !ENABLE_FEATURE_LOADFONT_PSF2
> #define do_loadtable(fd, inbuf, tailsz, fontsize, psf2) \
>         do_loadtable(fd, inbuf, tailsz, fontsize)
> #endif
> static void do_loadtable(int fd, unsigned char *inbuf, int tailsz, int
> fontsize, int psf2) {
> #if !ENABLE_FEATURE_LOADFONT_PSF2
> /* gcc 4.3.1 code size: */
> # define psf2 0 /* +0 bytes */
> //      const int psf2 = 0; /* +8 bytes */
> //      enum { psf2 = 0 }; /* +13 bytes */
> #endif

Dead code elimination can't change the number of arguments to functions, no. 
That's why I created the USE_BLAH() and SKIP_BLAH() macros way back when.
(What did they get renamed to...  IF_BLAH and IF_NOT_BLAH?  Ok, if that's
what you prefer...)

Let's see, quick stab at a cleanup... yup managed to do it with 0 bytes change.

function                                             old     new   delta
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0)                 Total: 0 bytes
   text	   data	    bss	    dec	    hex	filename
 893633	   4305	   9544	 907482	  dd8da	busybox_old
 893633	   4305	   9544	 907482	  dd8da	busybox_unstripped

Here's the patch:

diff --git a/console-tools/loadfont.c b/console-tools/loadfont.c
index e51142c..07e4fc9 100644
--- a/console-tools/loadfont.c
+++ b/console-tools/loadfont.c
@@ -55,7 +55,7 @@ struct psf1_header {
   && (x)->magic[1] == PSF1_MAGIC1 \
 )
 
-#if ENABLE_FEATURE_LOADFONT_PSF2
+// Stuff for ENABLE_FEATURE_LOADFONT_PSF2
 enum {
 	PSF2_MAGIC0 = 0x72,
 	PSF2_MAGIC1 = 0xb5,
@@ -86,7 +86,7 @@ struct psf2_header {
   && (x)->magic[2] == PSF2_MAGIC2 \
   && (x)->magic[3] == PSF2_MAGIC3 \
 )
-#endif /* ENABLE_FEATURE_LOADFONT_PSF2 */
+// End of stuff for ENABLE_FEATURE_LOADFONT_PSF2
 
 
 static void do_loadfont(int fd, unsigned char *inbuf, int height, int width, int charsize, int fontsize)
@@ -140,18 +140,10 @@ static void do_loadfont(int fd, unsigned char *inbuf, int height, int width, int
  * Some font positions may be described by sequences only,
  * namely when there is no precomposed Unicode value for the glyph.
  */
-#if !ENABLE_FEATURE_LOADFONT_PSF2
-#define do_loadtable(fd, inbuf, tailsz, fontsize, psf2) \
-	do_loadtable(fd, inbuf, tailsz, fontsize)
-#endif
-static void do_loadtable(int fd, unsigned char *inbuf, int tailsz, int fontsize, int psf2)
+static void do_loadtable(int fd, unsigned char *inbuf, int tailsz, int fontsize
+			IF_FEATURE_LOADFONT_PSF2(, int psf2))
 {
-#if !ENABLE_FEATURE_LOADFONT_PSF2
-/* gcc 4.3.1 code size: */
-# define psf2 0 /* +0 bytes */
-//	const int psf2 = 0; /* +8 bytes */
-//	enum { psf2 = 0 }; /* +13 bytes */
-#endif
+	IF_NOT_FEATURE_LOADFONT_PSF2(int psf2 = 0;)
 	struct unimapinit advice;
 	struct unimapdesc ud;
 	struct unipair *up;
@@ -164,14 +156,13 @@ static void do_loadtable(int fd, unsigned char *inbuf, int tailsz, int fontsize,
 
 	for (glyph = 0; glyph < fontsize; glyph++) {
 		while (tailsz > 0) {
-			if (!psf2) { /* PSF1 */
+			if (!ENABLE_FEATURE_LOADFONT_PSF2 || !psf2) { /* PSF1 */
 				unicode = (((uint16_t) inbuf[1]) << 8) + inbuf[0];
 				tailsz -= 2;
 				inbuf += 2;
 				if (unicode == PSF1_SEPARATOR)
 					break;
 			} else { /* PSF2 */
-#if ENABLE_FEATURE_LOADFONT_PSF2
 				--tailsz;
 				unicode = *inbuf++;
 				if (unicode == PSF2_SEPARATOR) {
@@ -198,9 +189,6 @@ static void do_loadtable(int fd, unsigned char *inbuf, int tailsz, int fontsize,
 				} else if (unicode >= 0x80) {
 					bb_error_msg_and_die("illegal UTF-8 character");
 				}
-#else
-				return;
-#endif
 			}
 			up[ct].unicode = unicode;
 			up[ct].fontpos = glyph;
@@ -218,7 +206,6 @@ static void do_loadtable(int fd, unsigned char *inbuf, int tailsz, int fontsize,
 	ud.entry_ct = ct;
 	ud.entries = up;
 	xioctl(fd, PIO_UNIMAP, &ud);
-#undef psf2
 }
 
 static void do_load(int fd, unsigned char *buffer, size_t len)
@@ -280,7 +267,8 @@ static void do_load(int fd, unsigned char *buffer, size_t len)
 	do_loadfont(fd, font, height, width, charsize, fontsize);
 
 	if (has_table)
-		do_loadtable(fd, table, buffer - table, fontsize, has_table - 1);
+		do_loadtable(fd, table, buffer - table, fontsize
+			IF_FEATURE_LOADFONT_PSF2(, has_table - 1));
 }
 

> Theretically, both #ifs are not necessary - compiler is able to deduce that
> psf2 is 0 if !ENABLE_FEATURE_LOADFONT_PSF2.

Sometimes you need to do seemingly redundant if (ENABLE_BLAH && variable) even
if the variable _should_ be 0 due to constant propogation, because the
compiler's not quite smart enough to figure it out for itself.  (You find that
sort of stuff out with make bloatcheck).

But manually triggering dead code elimination like that is still cleaner than
#ifdefs.

> Or rather, it is SUPPOSED TO be able to deduce it.
> I definitely see that gcc 4.3.1 is not really able to do it.
>
> The second #if is even more funny. literal 0, "const int" 0 and enum 0
> should be the same, right? Well, they are not.

You can take the address of an int, thus sometimes the compiler will allocate
space for it even if it doesnt' need to just because it's not smart enough to
figure out nobody will ever want to actually look at that memory.

I don't believe you can take the address of an enum. :)

> Basically, compilers are not as good as we'd want them to be,
> however they do gradually get better.

Hence the occasional need for if (ENABLE_BLAH && thisiszero) even when the
compiler should be able to figure it out.

Rob
-- 
Latency is more important than throughput. It's that simple. - Linus Torvalds