| From: Andrew Cagney <andrew.cag...@gmail.com> | On Thu, 20 Feb 2020 at 17:00, D. Hugh Redelmeier <h...@mimosa.com> wrote: | > | > | From: Andrew Cagney <andrew.cag...@gmail.com> | > | > | On Thu, 20 Feb 2020 at 15:59, D. Hugh Redelmeier <h...@mimosa.com> wrote: | > | > | > | > | From: Andrew Cagney <andrew.cag...@gmail.com> | > | | > | > If one declares all possible struct fd * things const, the absence of | > | > const highlights where references could go wrong. | > | | > | Why? | > | > Those are the only places that can (directly or indirectly) change a | > reference count. | | I don't follow. | | I attribute the bugs in the old code to it trying to be too clever by | minimising the number of places that a reference needed to be taken.
How so? I think that it had just the right number. | This resulted in code never being sure if it needed to take a | reference, or free the reference because it had encountered an error. You are anthropomorphising. Some programmers didn't understand the discipline (it was essentially like any non-auto resource allocation discipline). Clearly this was not documented well enough (by me). [I'm looking at freeswan-2.06 for the following.] In Pluto, there are a bunch of global variables that conceptually have the scope of a single transaction (where transaction is roughly what is done per-event in the event loop). The macro GLOBALS_ARE_RESET was meant to discover some mistakes with these variables. There is a cluster intended for providing context for logging. whack_log_fd was one of these per-transaction globals. You've replaced it with a bunch of parameters. The rules for duping the FD were fairly simple. If you wished to preserve the fd past the current transaction, dup it (because that fd would be closed at the end of the transaction). When is it preserved? - on initiation of a negotiation: the negotiation will span several transactions and each step needs to be logged. As a useful side-effect, the whack command will not complete until that fd gets closed. The whack fd will end up in the struct state. - similar case: when a pending negotiation is recorded. The whack fd will end up in the pending datastructure. - analogous case: when replacing an SA. - in a continuation for opportunistic or key_add This is a lot like the discipline needed for heap string references. | The new code takes a reference when ever the pointer is copied to/from | the heap (or heap to heap). | | If the object is made const and we make adding references harder we | just encourage code that incorrectly copies the actual pointer. These are harmless unless someone casts away the const. That assumes that const isn't used on a declaration that isn't auto (it generally should not be). _______________________________________________ Swan-dev mailing list Swan-dev@lists.libreswan.org https://lists.libreswan.org/mailman/listinfo/swan-dev