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

[Vadim Zeitlin]

On Wed, 8 Mar 2017 15:00:40 +0000 Greg Chicares <address@hidden> wrote:

GC> To reproduce:
GC> 
GC> File | New | Census
GC> Census | Edit case defaults
GC>   enter "nonblank" in 'Corporation Name' [required for group quote]
GC>   OK
GC>   answer 'Yes' to "Apply changes to every cell?"
GC> Census | Print group roster... | Print group premium quote to PDF
GC> 
GC> Kim observes the crash with msw-7. I do with 'wine'; a 'wine'
GC> backtrace is below.
GC> 
GC> Could this have anything to do with our recent wx upgrade?

 No, I think this is due to a bug introduced in lmi itself recently, please
apply this commit to fix it:
---------------------------------- >8 --------------------------------------
commit 2b5f868561212b6153f7372033cb64589cfe0938
Author: Vadim Zeitlin <address@hidden>
Date:   2017-03-08 16:54:46 +0100

    Fix bug in iteration over fields in group report code

    An extra iterator increment was accidentally added in the "Modernize for
    statements" commit (78614fe0c37678a28ccbb9439d887121b0aeeb7f), breaking
    the loop exit condition, which resulted in dereferencing an invalid
    iterator and a crash.

diff --git a/group_quote_pdf_gen_wx.cpp b/group_quote_pdf_gen_wx.cpp
index 6f9d080..f83c649 100644
--- a/group_quote_pdf_gen_wx.cpp
+++ b/group_quote_pdf_gen_wx.cpp
@@ -1229,7 +1229,7 @@ void 
group_quote_pdf_generator_wx::do_generate_pdf(wxPdfDC& pdf_dc)
     std::vector<extra_summary_field> const& f = report_data_.extra_fields_;
     fields.insert(fields.end(), f.begin(), f.end());

-    for(auto i = fields.begin(); i != fields.end(); ++i)
+    for(auto i = fields.begin(); i != fields.end();)
         {
         // Start a new table row and ensure it will be closed.
         open_and_ensure_closing_tag tag_tr(summary_html, "tr");
---------------------------------- >8 --------------------------------------

 I didn't have time to retest this using the official lmi build system, but
I did see this bug even in my build (thanks for providing, as usual, the
detailed instructions) and the commit above fixes it.

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

 Regards,
VZ