[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [lmi] Group quote PDF: segfault on strlen
From: |
Vadim Zeitlin |
Subject: |
Re: [lmi] Group quote PDF: segfault on strlen |
Date: |
Wed, 8 Mar 2017 23:10:09 +0100 |
On Wed, 8 Mar 2017 22:04:05 +0000 Greg Chicares <address@hidden> wrote:
GC> On 2017-03-08 21:18, Vadim Zeitlin wrote:
GC> [...]
GC> > It's not urgent to do anything about it, of course, but, just
GC> > for the record, I think there are 2 ways to improve things here:
GC> >
GC> > 1. Simple and direct: replace the loop with "for(auto const& f: fields)"
GC> > and then open/close the "tr" tag inside the loop, using a counter
GC> > (defined outside the loop, of course). The problem with this one is
GC> > that it would still to be simple to make a mistake with opening/closing
GC> > this tag and it would still be bad, even though (or maybe especially
GC> > because?) such a mistake wouldn't result in a crash any more (but
"just"
GC> > wrong output).
GC> >
GC> > 2. Higher level and more functional: iterate over pairs of fields, i.e.
GC> > replace the loop with "for(auto const& pair: view_by_pairs(fields))".
GC> > This would be, IMO, preferable, but does require writing our own
GC> > view_by_pairs() helper because we're not going to start using a big new
GC> > library just for this.
GC>
GC> What about...
GC>
GC> 3. Pad the container of fields (with something that will print as blank)
GC> as necessary in order to establish the invariant that the cardinality
GC> is always even. Then use simple, direct logic as in (1) above, except that
GC> the logic can be even simpler because we'd never print N-and-a-half pairs.
I don't see any material difference with (1). It's not the "if" on its own
which is the problem, it's that we need to open the <tr> tag before every
even index and close it after every odd one. As written now, this is
enforced using the open_and_ensure_closing_tag class and this nice property
would be lost if we switched to iterating over all the fields.
OTOH by iterating over pairs of fields we could *both* avoid dealing with
raw iterators, which is dangerous, as we've seen, *and* ensure that the
<tr> tags are always opened/closed correctly.
VZ