On 22.09.2011 22:25, Daniel Ruggeri wrote: > On 9/22/2011 5:39 AM, Kaspar Brand wrote: >> Having it in one patch seems fine to me, but in the end, it's the >> PMC members who will vote on backport proposals (IIUC), so it's >> their opinion which really matters. > > IINM, I believe we as committers all have a vote... that said, I hope > you would drop a +1 in the 2.2 STATUS file after the dust settles on > this change :-)
Hmm, I thought I wasn't supposed to cast votes on that, buy maybe I'm misinterpreting the guidelines (http://httpd.apache.org/dev/guidelines.html): > However, the only binding votes are those cast by active members of > the Apache Group; if the vote is about a change to source code or > documentation, the primary author of what is being changed may also > cast a binding vote on that issue. Maybe I'm somewhat confused by what "Apache Group" is actually referring to here - I read that to be the PMC... but I'll gladly stand corrected. Can someone clarify? > trunk suggestion - if this jives, I'll commit later when I have a bit Looks good, just some nits: > for (n = 0; n < ncerts; n++) { > int i, res; res is no longer used, AFAICT > if (chain != NULL) { > /* Dicard end entity cert from the chain */ > /* XXX: This is not needed if we collapse the two > * checks in ssl_engine_kernel in the future */ > X509_free(sk_X509_shift(chain)); s/Di/Dis/. As for the XXX, do you mean the idea of having a common routine for checking server certs and proxy client certs? That would probably go to ssl_engine_init.c as well, as sort of a companion to ssl_check_public_cert(). > else { > /* Discard empty chain */ > sk_X509_pop_free(chain, X509_free); > pkp->ca_certs[n] = NULL; Strictly speaking, the last assignment isn't necessary, since your calloc'ing ca_certs before. > if (i > 0) { > int j; > for (j=0; j<i; j++) { Style - missing spaces. Kaspar
