Current subsurface breakage

[Linus Torvalds]

On Fri, Jan 2, 2015 at 8:43 PM, Dirk Hohndel <dirk at hohndel.org> wrote:
>
> I reverted the temperature part and just pushed that out.

Ok, thanks, that fixes it.

And while you did that, I went ahead and tried to analyze a bit deeper
what exactly was going on. Because it turns out that the XML save
format was to be impervious to this problem.

And that, in turn, is because it turns out that the git loading and
the XML loading are subtly different.

With the git save format, on sample loading, we always start with the
previous sample state, except for cylinder pressure which we clear:

 * By default the sample data does not change unless the
 * save-file gives an explicit new value. So we copy the
 * data from the previous sample if one exists, and then
 * the parsing will update it as necessary.
 *
 * There are a few exceptions, like the sample pressure:
 * missing sample pressure doesn't mean "same as last
 * time", but "interpolate". We clear those ones
 * explicitly.

and the code literally does:

        if (sample != dc->sample) {
                memcpy(sample, sample-1, sizeof(struct sample));
                sample->cylinderpressure.mbar = 0;
        }

which matches the comment there.

HOWEVER. The XML parser is different, and rather than the above fairly
obvious code ("copy the whole old sample, then clear the cylinder
pressure") it does:

  static void sample_start(void)
  {
        cur_sample = prepare_sample(get_dc());
        cur_sample->ndl.seconds = lastndl;
        cur_sample->in_deco = lastindeco;
        cur_sample->stoptime.seconds = laststoptime;
        cur_sample->stopdepth.mm = laststopdepth;
        cur_sample->cns = lastcns;
        cur_sample->setpoint.mbar = lastpo2;
        cur_sample->sensor = lastsensor;
  }

  static void sample_end(void)
  {
        if (!cur_dive)
                return;

        finish_sample(get_dc());
        lastndl = cur_sample->ndl.seconds;
        lastindeco = cur_sample->in_deco;
        laststoptime = cur_sample->stoptime.seconds;
        laststopdepth = cur_sample->stopdepth.mm;
        lastcns = cur_sample->cns;
        lastpo2 = cur_sample->setpoint.mbar;
        cur_sample = NULL;
  }

so what happened was that since I use the git save format, any missing
temperatures in my save files would get populated as "same as the old
sample". Then we cleared duplicates, so when we saved, we saved the
same thing.

And then commit 6cf3787a0ed1 removed the clearing, so it would save
that changed state with all the duplicate temperatures.

But if you used the XML save format, any missing temperatures would
never have been initialized in the first place, so that commit made no
difference.

Anyway, I'd suggest applying the attached patch that makes the git
save format not save redundant temperatures. That actually makes your
partial revert be a non-issue.

I *also* wonder whether we should perhaps try to make the git and xml
saving logic be closer to each other. I think the git logic is perhaps
better in that it's simpler and doesn't special-case a lot of
different sample cases.

But that is a separate issue.

Hmm?

I'll take a look at the odd cylinder switch thing next.

                                   Linus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-git-save-format-Don-t-save-redunant-temperature-samp.patch
Type: text/x-patch
Size: 1571 bytes
Desc: not available
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20150102/0492f7b9/attachment.bin>