Re: [lmi] Group quote PDF: segfault on strlen

[Vadim Zeitlin]

On Wed, 8 Mar 2017 18:58:12 +0000 Greg Chicares <address@hidden> wrote:

GC> But actually this reminds me more of an airport novel I read once, where
GC> the night watchman at a covert Pu-processing facility saw a wastebasket
GC> full of metal shavings and thought it looked messy, so he took it outside
GC> and emptied it, because, well, what harm could that do?

[at the risk of totally losing any shred of my literary reputation that
 might have been remaining, I have no idea what was this about]

GC> >  FWIW I think this loop should be improved/rewritten to make it less
GC> > brittle, it's not really ideal that it got broken like this, but for now I
GC> > don't want to propose any bigger changes, knowing that you're preparing a
GC> > release candidate, so I made just this minimal patch to fix the bug.
GC> 
GC> I put a cautionary comment on it.

 Yes, thanks. It's not urgent to do anything about it, of course, but, just
for the record, I think there are 2 ways to improve things here:

1. Simple and direct: replace the loop with "for(auto const& f: fields)"
   and then open/close the "tr" tag inside the loop, using a counter
   (defined outside the loop, of course). The problem with this one is
   that it would still to be simple to make a mistake with opening/closing
   this tag and it would still be bad, even though (or maybe especially
   because?) such a mistake wouldn't result in a crash any more (but "just"
   wrong output).

2. Higher level and more functional: iterate over pairs of fields, i.e.
   replace the loop with "for(auto const& pair: view_by_pairs(fields))".
   This would be, IMO, preferable, but does require writing our own
   view_by_pairs() helper because we're not going to start using a big new
   library just for this.

 Please let me know if you'd like me to implement either of these
proposals,
VZ