[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [lmi] Group quote PDF: segfault on strlen
From: |
Greg Chicares |
Subject: |
Re: [lmi] Group quote PDF: segfault on strlen |
Date: |
Wed, 8 Mar 2017 18:58:12 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.6.0 |
On 2017-03-08 15:58, Vadim Zeitlin wrote:
> On Wed, 8 Mar 2017 15:00:40 +0000 Greg Chicares <address@hidden> wrote:
[...]
> 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:
[...]
> - for(auto i = fields.begin(); i != fields.end(); ++i)
> + for(auto i = fields.begin(); i != fields.end();)
This is certainly a careless mistake I made while applying part of your
pull/52 in January. I can't recover the state of mind that would have
led me to do this. I can only guess that I added "++i" here, intending
to remove it below, and then got interrupted by the doorbell and forgot
the second part of an atomic change. Without the imagined interruption,
that would still have been a mistake, but at least it would have been a
sane mistake to have intended to make.
But actually this reminds me more of an airport novel I read once, where
the night watchman at a covert Pu-processing facility saw a wastebasket
full of metal shavings and thought it looked messy, so he took it outside
and emptied it, because, well, what harm could that do?
> 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.
You nailed it. Thanks for sharing it right away--we can easily do the
remaining "official" steps.
> 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.
I put a cautionary comment on it.