httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: [PATCH] Balancers, VirtualHost and ProxyPass
Date Fri, 12 Dec 2014 10:56:57 GMT


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:jkaluza@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

Mime
View raw message