On 01/21/2016 08:03 PM, Markus Mayer wrote: > I left out the "flags" variable from the shm_open() call that you > showed, since I didn't see it being used or declared anywhere.
Sorry about that leftover from a previous version of my sketch. You did the right thing. > I did test it, and it works "as advertised". Your patch is solving two problems at once. You have described only one, but the other problem[1] is arguably more severe because (a) it leads to subtle bugs and crashes rather than startup failures and (b) most people run Squid on OSes other than OS X. [1] Item #1 at http://lists.squid-cache.org/pipermail/squid-dev/2015-December/004112.html IMO, we should document both problems in the code to minimize our chances of repeating them during future refactoring. I suggest replacing this comment: > + // OS X does not allow using O_TRUNC with shm_open. Also, OS X permits > + // ftruncate() to be called only once on a shared memory area. The call > + // will fail if the shared memory area was previously truncated. To > + // prevent this error, we delete and re-create the area if it existed > + // previously (i.e. from an unclean shutdown) and wasn't newly created. > + if (!createExclusive() && errno == EEXIST) { > + unlink(); > + createExclusive(); > + } with something like this (shown here without formatting): ``` Why a brand new segment? Our placement-new code requires an all-0 segment. We could truncate and resize an old segment, but OS X does not allow using O_TRUNC with shm_open() and does not support resizing after ftruncate(0). ``` and perhaps renaming createExclusive() to createFresh() for clarity. The above can be done during commit -- no need to repost IMO. Your commit message will explain the details. I (or another committer) will add text to state that problem [1] is also fixed by this change. If there are no objections, I will commit your patch (with the above polishing touches). Thank you, Alex. _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
