On Mon, Jun 26, 2017 at 3:14 PM, Jacob Champion <champio...@gmail.com> wrote: > On 06/20/2017 11:08 PM, William A Rowe Jr wrote: >> >> Sorry but I reraise my objection and veto worthless cpu cycles. > > For posterity, can I get a succinct description of your technical > justification for this veto?
I have several. * Wasting CPU cycles for a no-op *which the compiler cannot recover from is unacceptable. httpd is coded in C for a reason, with all the associated side effects. Any deoptimization as some matter of style or as a matter of security (e.g. stack -> alloc) is discussed on list and the costs weighed by the group. * The test_char.h table is a private-use entity with 256 well-defined octets (not utf-8, not char-wise, these are raw bytes). Sometimes, and often for some platform, one is wrong, it is simply fixed, and the sole consumer, util.c, is corrected. * The test_char.h table is private-use and is not exported, not installed to httpd target include/, etc. util.c was its sole consumer. * mod_log_forensic changed this and started consuming an include which was buried under server/ for its own devices. Questionable, but still a core module (changes to the core table less likely to be caught when reviewing the module, but the toggle is clearly labeled FORENSIC, so there's that. Note the module duplicates the entire table, but it's forensic/diagnostic so I don't care what such code and const static footprints look like.) * T_HTTP_TOKEN_STOP declared NUL did not stop a token. That is an idiotic assertion. I inverted it. Cursory examination of the code, I did not spend enough time studying side-effects of the end-of-string behaviors, nor did the many other reviewers, I looked at the immediate affected logic. All use of this array and it's value of element NUL are exercised in util.c, this is not like it was some obscure external citation. * I agree with you the entire thing is unclear because there is no entity called a TOKEN_STOP, although there are recognized separators, and non-token garbage entities, but the functions implicated never bothered to distinguish between any of this. T_HTTP_TOKEN is clearer and less ambiguous (and NUL does not belong in that set.) I'm about to propose that change and will VOTE DOWN any call to backport that change to 2.4, as... * This change is mutually agreed to be a no-op, and you've made clear the intent was to backport to a stable, maintenance branch which should see NO deltas which have no effect. For example, whitespace correction is absolutely prohibited on a release branch because the svn blame ... comprehension of the changes to the code is destroyed by a stupid endeavor to make things look nice. When whitespace changes are introduced, it is in order to port the minimal, identical changes previously applied to trunk, so that a functional patch can be applied cleanly from an svn merge. So to convince me my veto is unwarranted, you need to basically convince me that the array elts are too volatile to trust, including the elt[0], or that we have a long tradition of getting this wrong (I suggest we don't - this was an exception), that we are unable to maintain the pairing util.c <> test_char.h. OR that we are about to export test_char.h to a much less attentive audience as a public export, etc.
