[PATCH] Air isn't special

Linus Torvalds torvalds at linux-foundation.org
Sun Jun 1 11:40:01 PDT 2014


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.

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.

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.

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.

               Linus


More information about the subsurface mailing list