httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sander Temme <scte...@apache.org>
Subject Re: svn commit: r406716 - /httpd/httpd/branches/2.2.x/STATUS
Date Sat, 20 May 2006 21:11:41 GMT
Joe,

On May 17, 2006, at 7:42 AM, Joe Orton wrote:

Thanks for your review.

> On Mon, May 15, 2006 at 06:56:35PM -0000, sctemme@apache.org wrote:
>> Author: sctemme
>> Date: Mon May 15 11:56:34 2006
>> New Revision: 406716
>>
>> URL: http://svn.apache.org/viewcvs?rev=406716&view=rev
>> Log:
>> Propose backport of ServerName directive enhancement
>
> Hey, let's put new features in the stable branch!
>
> 1) CHANGES entries should be first and foremost readable by your  
> average
> server admin; I'd suggest two entries, one which is admin-readable,  
> one
> which is aimed at the module developer:
>
>   *) Add optional 'scheme://' prefix to ServerName directive,
>      allowing correct determination of the canonical server URL
>      for use behind a proxy or offload device handling SSL; fixing
>      redirect generation in those cases.
>
>   *) Added server_name field to server_rec for above.  Minor MMN bump.

server_scheme field. Picked those up.

> The fact that you tested the change is certainly not something that
> needs documenting in CHANGES.  There's also a PR for this bug I think
> somewhere... PR 33398, which should be referenced (and closed, since
> it's unlikely to be fixed for 1.3).

Yeah, 1.3 without EAPI can't do this since the scheme is hardcoded.  
EAPI at least makes it somewhat settable, but we can't fix this in  
the base distribution.

Perhaps Phil Dibowitz can upgrade to 2.2...

> 2) this introduced a compiler warning so the fix for that needs
> backporting too at least (r405010)

Yeah, the unused info pointer. Zapped.

> 3) for a known-length pstrndup use pstrmemdup not pstrndup when you  
> know
> there is no NUL in the length being dup'ed; this is wrong twice in the
> core.c changes

Lines 2331 and 2341. Fixed.

> 4) again in core.c:
>
>         cmd->server->server_scheme = (const char *)scheme;
>
> that looks like an unnecessary cast.

Yeah, that is superfluous.

I have uploaded a new version of the patch to

  http://people.apache.org/~sctemme/servername_22x.patch

for further perusal.

S.

-- 
sctemme@apache.org            http://www.temme.net/sander/
PGP FP: 51B4 8727 466A 0BC3 69F4  B7B8 B2BE BC40 1529 24AF



Mime
View raw message