On Thu, 23 May 2019 at 07:20, Antony Antony <[email protected]> wrote: > > Hi Andrew, > > thanks for the heads up. > > wow, that is an interesting change! At first glance it appears quite > different from the Feb 22 proposal. I guess this is better! > I say push it now, sooner than later.
There was general push back on my suggestion to copy FS into state, and a preference for flushing the compat macros as they just made code confusing (and debugger support still isn't really there). > I am wondering "st_fs" instead "st_state" would be better? your call! You > probably evaluated shorter names and discarded them:) I wondered about that; along with .st_fs->fs_... Main reason for st_state is to preserve a name we know (annoyingly the compiler doesn't complain about st->st_state == STATE_UNKNOWN so I had to find them manually). > Would you be able to do it in one commit? > I think this change will not to affect any output, or test case reference > outputs. However, IMHO the code change is pretty big. > This is why I am in favor to push it now than waiting. Otherwise it would be > painful for you and others. It's one commit. > Lets start a new thread to serialize changes the usual suspects want to push > post 3.28, in the next couple of weeks. Some of us have 6 months of > finished, or almost finished branches to push. Such as xfrmi, Andrew O(1) > patches, this one.. > > -antony > > > On Wed, May 22, 2019 at 09:09:39PM -0400, Andrew Cagney wrote: > > Heads up. > > > > I'm about to push a change renaming .st_finite_state to .st_state; > > inline all the wrapper macros such as .st_state_name; and drop the > > .fs_ prefixes. While a lot of code gets cleaned up vis: > > > > - enum_name(&state_names, > > - st->st_state > > - ))); > > + st->st_state->name)); > > > > and: > > > > - lswlogf(buf, "%s: %s", > > st->st_finite_state->fs_name, > > - st->st_finite_state->fs_story); > > + lswlogf(buf, "%s: %s", st->st_state->name, > > + st->st_state->story); > > > > > > it doesn't try to fix code like: > > > > - bool responder = (st->st_state != STATE_PARENT_I2); > > + bool responder = (st->st_state->kind != STATE_PARENT_I2); > > > > where the "correct fix" is to instead use attributes such as > > st->st_sa_role or md->message_role. Later for that. > > > > On Thu, 21 Feb 2019 at 10:40, Antony Antony <[email protected]> wrote: > > > > > > Hi Andrew, > > > > > > On Wed, Feb 20, 2019 at 09:22:52AM -0500, Andrew Cagney wrote: > > > > This continues a face-to-face discussion from last year. > > > > > > I re-collect such a discussion from last fall. > > > If you are thinking of fixing only because of our discussion, then please > > > do > > > not change. > > > > > > I tried to convey my annoyance of unexpected change of a > > > well known variable to a #define! The new one was hard to use in gdb. > > > It is long, "st->st_state" vs "st->st_finite_state->fs_kind". > > > Also code used short version at many places, mixed usage was annoying. > > > Now mostly the long version. If we are not mixing them or rename every 6 > > > months I am ok:) > > > > > > I was suggesting to replace all instances of short form at once. However > > > when I talked to you I got the impression that your preference was to > > > change > > > inclemently. And re-write often! The use st->st_state is disappearing now? > > > > > > f937038e9d > > > +#define st_state st_finite_state->fs_state > > > The commit was over a year ago:) > > > > > > Over time the use of st->st_state shrunk. However, more variants > > > appeared:) > > > see 179bf3901. I prefer one version for a well know variable name. It is > > > probably a matter of taste! > > > > > > > It was pointed out that one downside of replacing 'enum state_kind' > > > > with 'struct finite_state' is that when a 'struct state' is printed > > > > using a debugger it no longer shows the 'state' as an enum. > > > > > > And now this proposal sounds like just when I am getting used to the long > > > form there may be another change. Thanks for the heads up! > > > > > > > Off hand I can think of two solutions: > > > > > > > > - redundantly store both a 'struct finite_state' pointer, and an 'enum > > > > state_kind' in 'struct state' > > > > > > > > - store a copy of 'struct finite_state' in 'struct state' > > > > > > > > My preference is for the second > > > > > > my preference is fewer "defines" for well known variables. such as > > > st_state > > > or say st_serial_no. > > > > > > _______________________________________________ > > > Swan-dev mailing list > > > [email protected] > > > https://lists.libreswan.org/mailman/listinfo/swan-dev _______________________________________________ Swan-dev mailing list [email protected] https://lists.libreswan.org/mailman/listinfo/swan-dev
