From: Ingo Schwarze <firstname.lastname@example.org> To: "Anthony J. Bentley" <email@example.com> Cc: firstname.lastname@example.org Subject: Re: Improper formatting of table with T& Date: Mon, 29 Mar 2021 02:33:12 +0200 [thread overview] Message-ID: <YGEgSHjG98FpkvOemail@example.com> (raw) In-Reply-To: <YGDo6DpTQu0p3fDY@asta-kit.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 firstname.lastname@example.org
next prev parent reply other threads:[~2021-03-29 0:33 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-06 4:54 Anthony J. Bentley 2021-03-28 20:36 ` Ingo Schwarze 2021-03-29 0:33 ` Ingo Schwarze [this message] 2021-04-07 9:34 ` Anthony J. Bentley
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YGEgSHjG98FpkvOemail@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: Improper formatting of table with T&' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).