Re: Improper formatting of table with T&

[Ingo Schwarze <schwarze@usta.de>]

Hi,

.. and by the way, after fixing the bug, i got worried there might
be something i'm missing and there might be more problems hiding here
because i wondered how it could possibly happen that before the patch i
just sent to you, your (Anthony's) version of the test file (with the T&)
exhibited the bug and my changed version (without the T& and the final
layout line pulled up to the main layout block) worked.  The *final*
layout data structure ought to be just identical on both cases...
So what was going on?

The key is a few lines down in tbl_data.c, function tbl_data(),
the top of which i patched.  That function is actually used for
parsing *any* data line, both those containing data and those
containing only horizontal rulers.  So consider what happens
when the *ruler* is processed right after the title line of the
table.

Without my patch and with Anthony's version of the test file,
sp->layout->next is NULL (because the T& has not yet been reached)
so rp gets set to sp->layout (the layout line for the title),
and then that gets used for initializing the span struture for
the ruler, a few lines down in the switch statement.  So the
ruler gets associated with the *header* layout line and that
doesn't change when the T& is later parsed.

On the other hand, with my changed test file, the whole layout structure
including the layout for data lines is already present when the ruler
is parsed, so sp->layout->next is *not* NULL but the final layout line.
Hence the ruler gets associated with the layout line intended for data.
Of course, it doesn't matter at all which layout line is associated with
a ruler, no layout is needed for rendering a ruler in the first place,
so the difference doesn't matter for rendering the ruler itself.

But then later, when parsing the first data line, the one for "days",
the bug caused the meaningless layout pointer stored with the ruler
to actually get used: the incorrect condition "sp->pos == TBL_SPAN_DATA"
in the rp assignment line was false in this case, so whatever garbage
pointer was there got used.  That resulted in correct formatting (by
chance) for my test file but blew up for Anthony's test file.

I'm no longer convinced storing such meaningless pointers is a smart move,
precisely because there is a risk something might use them down the line,
causing erratic behaviour that is hard to disentangle.  But at least i'm
now confident i understand what was going on, and short of redesigning
the invariants of struct tbl_span (which might improve rigour at the
risk of causing regressions) i'm now confident my patch is correct.

Yours,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mandoc.bsd.lv