Re: non-timely dpkg question

[Ian Jackson <ian@chiark.greenend.org.uk>]

Klee Dienes writes ("non-timely dpkg question"):
> I should figure this out for myself, but I'm too lazy:

I'm afraid I had to RTFS, having largely forgotten the details, but
here we go ...

> Where in dpkg does the second 'calls' field in 'struct cleanupentry'
> ever get used?  So far as I can tell, all of the functions which call
> push_cleanup pass 'NULL' for call2 and '0' for mask2.

It looks like the second call/mask pair is just a convenient way to
stack two callbacks at once.  ISTR that there used to be places where
one cleanup would be pushed for `success' returns and a different one
for errors.

In fact, in 1.4.0.17, processarc.c contains this:
      push_cleanup(cu_prermdeconfigure,~ehflag_normaltidy,
                   ok_prermdeconfigure,ehflag_normaltidy,
                   3,(void*)deconpil->pkg,(void*)conflictor,(void*)pkg);

I don't have easy access to a more recent tree ATM, but I would be
surprised (and worried!) if someone had changed this call.  I
certainly haven't.

Perhaps it would be cleaner if the multiple-entries per cleanupentry
were phased out, but if it ain't broke don't fix it, and I don't even
think it's really that messy.

> If the second field does in fact never get used, what is the purpose
> of push_checkpoint()?  It would seem from the implementation of
> pop_cleanup that the cpmask and cpvalue only get applied to the
> seconde 'calls' field.  Or should I assume this was a bug in the
> implementation of checkpointing and the mask was inteded to apply to
> all previous cleanups?

The comment at the top of push_checkpoint says:
  /* This will arrange that when error_unwind() is called,
   * all previous cleanups will be executed with
   *  flagset= (original_flagset & mask) | value
   * where original_flagset is the argument to error_unwind
   * (as modified by any checkpoint which was pushed later).
   */

Note that a struct cleanupentry will have _either_ mask/call pairs (an
entry made by push_cleanup) _or_ cpmask and cpvalue (an entry made by
push_checkpoint).  Therefore, I think the implementation in
pop_cleanup will behave correctly in the current environment.
However, it is rather strange, and would have silly semantics if there
were a cleanupentry with both, and be broken if the modification to
flagset were not idempotent.

The modification to flagset should probably be after the i loop
instead of inside it.  If you (or someone else who thinks they
understands the code) concur then such a change should probably be
checked in; if noone does then I may look at it again to make sure I'm
not going mad and then do it myself.

Ian.


--
To UNSUBSCRIBE, email to debian-dpkg-request@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org