[PATCH] no config file overwrite for dpkg

Rob Landley rob at landley.net
Fri Feb 19 14:32:02 UTC 2010


On Thursday 18 February 2010 12:05:51 Mike Frysinger wrote:
> On Thu, Feb 18, 2010 at 11:13 AM, Kim B. Heino wrote:
> > --- orig-1.16.0/archival/Config.in      2010-01-25 02:59:38.000000000
> > +0200 +++ busybox-1.16.0/archival/Config.in   2010-02-18
> > 17:59:54.274564790 +0200 @@ -128,6 +128,15 @@
> >          This implementation of dpkg has a number of limitations,
> >          you should use the official dpkg if possible.
> >
> > +config FEATURE_DPKG_NO_CONFIG_OVERWRITE
> > +       bool "Don't overwrite existing config files"
> > +       default n
> > +       depends on DPKG
> > +       help
> > +         By default dpkg will overwrite existing config files when
> > +         upgrading. With this feature enabled new config files will be
> > +         created as filename.dpkg-new.
> > +
> >  config DPKG_DEB
> >        bool "dpkg_deb"
> >        default n
> > diff -ur orig-1.16.0/archival/dpkg.c busybox-1.16.0/archival/dpkg.c
> > --- orig-1.16.0/archival/dpkg.c 2010-01-25 02:59:38.000000000 +0200
> > +++ busybox-1.16.0/archival/dpkg.c      2010-02-18 18:05:34.983814610
> > +0200 @@ -1489,6 +1489,21 @@
> >        return ar_handle->dpkg__sub_archive->dpkg__buffer;
> >  }
> >
> > +#if ENABLE_FEATURE_DPKG_NO_CONFIG_OVERWRITE
> > +static char FAST_FUNC filter_rename_config(archive_handle_t
> > *archive_handle) +{
> > +       struct stat dummystat;
> > +       char *name_ptr = archive_handle->file_header->name + 1;
> > +
> > +       if (find_list_entry(archive_handle->accept, name_ptr) &&
> > +           stat(name_ptr, &dummystat) == 0) {
> > +               printf("Warning: Creating %s as %s.dpkg-new\n", name_ptr,
> > name_ptr); +               archive_handle->file_header->name =
> > xasprintf("%s.dpkg-new", archive_handle->file_header->name); +       }
> > +       return EXIT_SUCCESS;
> > +}
> > +#endif
> > +
> >  static void FAST_FUNC data_extract_all_prefix(archive_handle_t
> > *archive_handle) {
> >        char *name_ptr = archive_handle->file_header->name;
> > @@ -1508,6 +1523,10 @@
> >        if (name_ptr[0] != '\0') {
> >                archive_handle->file_header->name = xasprintf("%s%s",
> > archive_handle->dpkg__buffer, name_ptr);
> > data_extract_all(archive_handle);
> > +#if ENABLE_FEATURE_DPKG_NO_CONFIG_OVERWRITE
> > +               if (fnmatch("*.dpkg-new",
> > archive_handle->file_header->name, 0) == 0) +                      
> > archive_handle->file_header->name[strlen(archive_handle->file_header->nam
> >e) - 9] = 0; +#endif
>
> the point of the ENABLE_XXX stuff isnt so that you can do:
> #if ENABLE_XXX
>
> it's so that you can do:
> if (ENABLE_XXX)
>
> please convert he style in this patch
>
> maybe we should FAQ this ...

Agreed.  (I actually plan to send a bunch of cleanup patches upstream someday 
for some of the existing nightmares, like top.)

As long as we're proposing a FAQ:

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
  }

Imposes no extra runtime overhead, because an always-true constant test gets 
optimized away.  (This includes if (1 && realtest) becomes just if (realtest), 
or becomes if (0) and the whole block discarded.)

The useful difference is that #if chops out code before the compiler gets a 
chance to syntax-check it, leading to bit-rot.  It also means you jump through 
hoops to get your curly brackets to match up, it encourages you to be lazy 
about keeping indentation levels consistent, it tempts you to put local 
variable declarations  in the middle of a curly-bracket block level when 
they're local to an #ifdef block, and it's just plain harder to read.

Using if (0) means that changes you make that impact other configurations than 
the one you're using will at least be compile-tested, and what indentation 
level the code should be at is obvious.

It also means that your logic is in one place:

#ifdef BLAH
  int variable;
#endif
  code code code
#ifdef BLAH
  do  {
#endif
  code code code
#ifdef BLAH
  } while (loop_on(&variable));
#endif

vs

  int variable;  // discarded by dead code elimination if unused.

  code code code

  do {
    code code code
  } while (BLAH && loop_on(&variable));

Nice simple C, some of which is evaluated at compile time.

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


More information about the busybox mailing list