On 12/12/2014 09:44 AM, Jan Kaluža wrote: > On 12/11/2014 03:05 PM, Plüm, Rüdiger, Vodafone Group wrote: >> >> >>> -----Original Message----- >>> From: Jan Kaluža [mailto:jkal...@redhat.com] >>> Sent: Donnerstag, 11. Dezember 2014 14:40 >>> To: dev@httpd.apache.org >>> Subject: Re: [PATCH] Balancers, VirtualHost and ProxyPass >>> >>> On 12/11/2014 08:47 AM, Jan Kaluža wrote: >>>> On 12/10/2014 08:21 PM, Ruediger Pluem wrote: >>>>> >>>>> >>>>> On 12/10/2014 02:21 PM, Jan Kaluža wrote: >>>>>> On 12/10/2014 01:49 PM, Plüm, Rüdiger, Vodafone Group wrote: >>>>>>> But this way we lose the base ones that are not touched in the >>>>>>> virtual host and e.g. are only used by rewriterules. >>>>>>> So we should transfer the base ones to the merged array in any case >>>>>>> and update them where needed. >>>>>> >>>>>> Hm, you are right. Check the new version attached to this email. >>>>> >>>>> But this one changes the parent configuration. So if you have two >>>>> virtual hosts and in each you change a different >>>>> parameter of the parent the later one has *both* changes, not only >>>>> one. So you would need to do a copy of these >>>>> balancers first and adjust the copy. Then only add the adjusted copy >>>>> to the result. For the base balancers not matching >>>>> the override balancers just add them to the result untouched. Same for >>>>> the override ones, as you already do >>>> >>>> That makes sense, thanks. I will work on it and send updated patch. >>> >>> I have to admit that this feature is starting to be more time-consuming >>> than I thought :). The attached patch should address all the issues >>> pointed by Yann and Rüdiger. >> >> Looks fine in general. Details: > > I hope I've finally fixed everything now :), see the attached patch please. I > really appreciate your reviews here!
Looks good. One thing though: I think apr_array_cat is still wrong since we only copied a *pointer* to the array with *b2 = *b1. So apr_array_cat adjusts the original array from the base. I guess we should do: apr_array_cat(tmp.errstatuses, b2->errstatuses); b2->errstatuses = tmp.errstatuses; Of course this is only fine when the order of the errstatuses array doesn't matter. Otherwise: b2->errstatuses = apr_array_append(b2->errstatuses, tmp.errstatuses); And from order perspective, maybe: return apr_array_append(p, tocopy, overrides); If order doesn't matter I guess apr_array_cat(overrides, tocopy); return overrides; would be also fine. Regards Rüdiger
