<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div dir="ltr"><h2><span style="font-weight:normal"></span><br></h2><div class="gmail_extra"><div class="gmail_quote">2014-07-03 10:20 GMT+02:00 Rémi Denis-Courmont <span dir="ltr"><<a href="mailto:remi@remlab.net" target="_blank">remi@remlab.net</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Le 2014-07-02 23:48, David Fuhrmann a écrit :<div><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Ping for ideas, and opinions about the existing patch(es)?<br>
</blockquote>
<br></div>
None of the developers who wrote this code are still active, so it should come to you as no surprises that asking for explanations elicits no useful answers.<br>
<br>
Either you rewrite your patch in a way that makes sense, or you leave the code as is.<span><font color="#888888"><br></font></span></blockquote><div><br></div><div>Hello all,<br><br></div><div>I think that the patch make some sense, and I already explained why in my last mail in detail.<br>
<br></div><div>Especially because nobody of the original developers is still active, I would like to also get the opinions from other active developers. If it is common sense also among other devs that the patch is too hackish, it is fine with me.<br>
<br>But if it is only you that basically criticize that "the patch look fishy", and it "probably does not fix the root of the problem", then this is not really an acceptable reason for me to reject the patch. I already tried to explain in detail why I think that the patch should be applied at least as an intermediate solution. Unfortunately, for me it looks like you ignored most of my explanations, and gave no more concrete objections to those.</div><div><br></div><div>In summary: If you look at at the code just some lines above the patch it is inevitable that the unsigned underflow can happen, and this case is already sort of taken into account in the modified error path. What was missing is setting the internal state to sane values again - which is common practice for error paths where the state is accessed afterwards.<br>
<br></div><div>The underlying bug is reproducible and visible, with different files and on different OS and hardware versions. Thus, I see the bug as a blocker for the 2.2 release. Because of the complexity of the underlying code and lack of time and code knowledge, I do not see that I (or felix) will find a better solution soon.</div><div><br></div><div>So its better to avoid an uncontrollable crash instead of leaving it open, if it will not properly fixed in the near future. Due to the invalid memory access it might be even supposable that this forms a security vulnerability, but I did not further checked that.</div><div>
<br></div><div>Because of the mentioned reasons, I think that the decision about the patch should not be done only by you (Remi), but also involving opinions of the other core devs.<br><br></div><div>For reference, I attached the minimal patch with is needed to fix the bug (#11488) and which is in question, basically.<br>
<br></div><div>With best regards,<br></div><div>David<br>
</div><div><br></div></div></div></div></body></html>