lmi
[Top][All Lists]
Advanced

[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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]