[Swan-dev] cur_state and friends [was Re:Fwd: Changes to ref refs/heads/master]

D. Hugh Redelmeier hugh at mimosa.com
Tue Jan 16 18:54:28 UTC 2018


[All this is from fallible memory since archaeology is
labour-intensive.]

| From: Andrew Cagney <andrew.cagney at gmail.com>

| Yea, this is part of a more general problem.  But first some
| background.  We've always kind of sort of always had the stack:
| 
| - cur_state | ... | cur_state | cur_connection | cur_from

The original design apparently wasn't robust in the face of 20 years
of maintenance.  Either a little two intricate or not documented well
enough.

There really was no stack.  But there was a priority.  If cur_state was 
set, that was used to prefix logging.  Otherwise, if cur_connection was 
set, that was used to prefix logging.  cur_* were ONLY supposed to be used 
for logging.

set_cur* were meant to be idempotent.  Certainly not a feature of stack 
pushing.  Idempotence makes correctness easier (eg. the change to
timer_event_cb would not be required).

Some (not many) routines needed to temporarily switch cur_state.  It was 
their job to save the old cur_state value and later restore it.  If this 
were common, a stack would make sense.  With the transaction-processing 
design of Pluto, this was not at all common.

There is a particular hazard with a routine privately saving a copy
of cur_state.  If that old state were deleted, who would prevent that
dangling pointer being re-installed as cur_state?  I don't think that
case could come up, but who knows with all the code changes that have
happened.

One of my biggest sins was to use cur_state to smuggle the current state 
around some of the crypto code.  It was meant to be a temporary hack until 
I got the next chunk of changes done and committed.  But that didn't 
happen.  I think Andrew fixed that recently with some refactoring.

| - when a state gets deleted, there's general confusion over who should
| be pushing/popping cur_state

In the original design, only one thing ever deleted a state:
delete_state().

If cur_state pointed to the state to be deleted, delete_state would
set it to NULL.  But it didn't do that immediately since some of its
diagnostics were usefully prefixed.

Same pattern for cur_connection.

With a stack, things get trickier.  It's nice for things that act like
brackets dynamically (i.e. in execution) to appear as brackets
statically (i.e. in the code).  That's not often the case with
push_cur_state and pop_cur_state.


More information about the Swan-dev mailing list