> -----Original Message----- > From: p...@golemon.com [mailto:p...@golemon.com] On Behalf Of Sara > Golemon > Sent: Tuesday, December 19, 2017 10:05 PM > To: Anatol Belski <a...@php.net> > Cc: PHP internals <internals@lists.php.net> > Subject: Re: [PHP-DEV] C++ and FAST_ZPP macros > > On Tue, Dec 19, 2017 at 3:29 PM, Anatol Belski <a...@php.net> wrote: > >> As the blog post notes, it's a simple matter to work around the bug > >> in extension code (indeed, an extension can simply opt to not use > >> FAST_ZPP). On the other hand, the fix is pretty basic, and existing > >> functionality of the default expected type effectively being > >> Z_EXPECTED_LONG (because both have a value of zero) is just a bit.... > >> weird. > >> > >> Thoughts? If I don't hear anything in a week, I'll just apply to 7.1 and > >> merge > up. > >> > > C++11 requires a validity scope for enums, thus this change is unavoidable. > > IMO your second suggestion were safer in the spirit of BC - > > Z_EXPECTED_LONG instead of IS_UNDEF by default, and a new in master. > > Just because a workaround is possible and otherwise the issue is not > > critical. > > > Yeah, the more I think about it, the more I realize that's the more prudent > approach given the non-critical nature of this. Could you clarify as 7.0 RM > if you > want this on your branch? AIUI we're in security-only for 7.0 now, yes? > In case of 7.0, it's a border case now. 7.0.27RC1 is out, the final is not yet. Strictly speaking, this issue doesn't qualify as a last second fit. Nevertheless I'd say, replacing IS_UNDEF with Z_EXPECTED_LONG is totally fine in C89 as it's only aware of the actual value, the issue in latest C++ is fixed thereby. From Jordi's stats 2017 is to see, that 7.0 was 1/3 in January at least, perhaps less today, so in general it'd be still some goal projects would target. As a last second patch, I think the nonintrusive variant would be acceptable, as C++ standard moves forward an we'd see ever more usage even of C++14 much later this year. If no one sees an obvious issue, please merge into 7.0 next days. I'll be testing subsequently as well with any possible exts. 7.0.27 GA is the cut.
Thanks Anatol
