<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 23 January 2016 at 09:35, Alex Rousskov <span dir="ltr"><<a href="mailto:rousskov@measurement-factory.com" target="_blank">rousskov@measurement-factory.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On 01/21/2016 08:03 PM, Markus Mayer wrote:<br>
<br>
> I left out the "flags" variable from the shm_open() call that you<br>
> showed, since I didn't see it being used or declared anywhere.<br>
<br>
</span>Sorry about that leftover from a previous version of my sketch. You did<br>
the right thing.<br>
<span><br>
<br>
> I did test it, and it works "as advertised".<br>
<br>
</span>Your patch is solving two problems at once. You have described only one,<br>
but the other problem[1] is arguably more severe because (a) it leads to<br>
subtle bugs and crashes rather than startup failures and (b) most people<br>
run Squid on OSes other than OS X.<br>
<span><br>
[1] Item #1 at<br>
<a href="http://lists.squid-cache.org/pipermail/squid-dev/2015-December/004112.html" rel="noreferrer" target="_blank">http://lists.squid-cache.org/pipermail/squid-dev/2015-December/004112.html</a></span></blockquote><div> </div><div>Right. I completely forgot about that. I had most of the documentation written up before I learned about the other problem.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">IMO, we should document both problems in the code to minimize our<br>
chances of repeating them during future refactoring. I suggest replacing<br>
this comment<br></blockquote><div><br></div><div>Agreed.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">```<br>
Why a brand new segment? Our placement-new code requires an all-0<br>
segment. We could truncate and resize an old segment, but OS X does not<br>
allow using O_TRUNC with shm_open() and does not support resizing after<br>
ftruncate(0).```<br></blockquote><div><br></div><div>Maybe say "doesn't support resizing using a second ftruncate() call" or something along those lines, since it doesn't support resizing of any kind, not just after ftruncate(0). But I'm not going to insist on you rewording the comment you have.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">and perhaps renaming createExclusive() to createFresh() for clarity. The<br>
above can be done during commit -- no need to repost IMO.<br><br>
Your commit message will explain the details. I (or another committer)<br>
will add text to state that problem [1] is also fixed by this change.<br></blockquote><div><br></div><div>No objections. Looks good to me.</div><div><br></div><div>Regards,</div><div>-Markus</div><div> </div></div></div></div>