[PATCH] Air isn't special

[Dirk Hohndel]

On Sun, Jun 01, 2014 at 11:40:01AM -0700, Linus Torvalds wrote:
> On Sun, Jun 1, 2014 at 10:37 AM, Dirk Hohndel <dirk at hohndel.org> wrote:
> >>
> >> those are actually sane and simple rules.
> >
> > That does seem sane. Given it's late in the Linux cycle, do you have the
> > time to send a patch that consistently does that everywhere?
> 
> The problem is, our "struct gasmix" is actually nice and easy to use,
> and using "get_o2()" and friends on it is fairly simple and obvious.
> 
> BUT
> 
> .. and then we break that nice easy encapsulation in tons and tons of
> places, most notably when it comes to events.

Funny you should mention that -- I am mostly done with fixing that.
Once I push this out, you run get_gasmix_from_event(ev) and you get back a
nice gasmix... which you can then use get_o2 and get_he on :-)

> The event encoding of gas mixes is easily my least favorite part of
> our event handling. It would be lovely if the event structure
> containted a "struct gasmix", but it doesn't. As you know, it contains
> a integer "value" that has the o2/he encoded not as permille, but as
> percentages, and in the low/high 16 bits of that one integer.
> 
> And partly because of issues like this, we don't actually pass "struct
> gasmix" around at all, instead we pass around "int o2" and "int he"
> separately to the planner, for example. Look at "add_plan_segment()",
> for example, but also look at things like this:
> 
>         get_gas_from_events(&dive->dc, t0, &o2, &he);
>         if ((gasidx = get_gasidx(dive, o2, he)) == -1) {
>                 report_error(translate("gettextFromC", "Can't find gas
> %d/%d"), (o2 + 5) / 10, (he + 5) / 10);
>                 gasidx = 0;
>         }
> 
> iow, we get the o2/he values as integers (converted to permille) from
> the event (which had it in that odd "encoded two percentages in one
> integer" form), then try to find the closest matching gas index, and
> then we later use &dive->cylinder[gasidx].gasmix in order to pass the
> gasmix into the interpolate_transition() code.

Which is the reason why I'm fixing that part. So we pass gasmix around
instead.

> Anyway, I wouldn't touch "get_o2()" with a ten-foot pole before doing
> a lot of other cleanups first. Because as it is, we actually _depend_
> on get_o2() returning an integer permille, because we end up passing
> it around as such to various planner functions.
> 
> So the first cleanup would probably have to be to try to keep things
> as "struct gasmix" as long as possible. But it might be painful. The
> planner in particular does *not* tend to use our nice type-safe
> structures, it passes just "int" around, having dropped the type
> information. (this is not limited to gasmix, btw, it's true of things
> like depth etc - the planner tends to take "int" rather than
> "depth_t", and the fact that it is mm is kind of implied.

I have started addressing that as well, as you may have seen in my last
few commits. Slowly but surely I'm trying to get us back to our nice
types.

> Sadly, type cleanups tend to be *really* painful, because the type
> percolates all the way, and we have some helper functions that
> explciitly don't take the typed structures because they end up being
> used by code that doesn't _have_ the typed structures any more..
> 
> I can try to look at some of it.

I hate saying this to any contributor... but wait a little bit and maybe
this will become easier.

/D