httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Plüm, Rüdiger, VF-Group <ruediger.pl...@vodafone.com>
Subject Re: [PATCH 43415] Logging remote port.
Date Wed, 19 Sep 2007 09:47:03 GMT


> -----Ursprüngliche Nachricht-----
> Von: Adam Hasselbalch Hansen 
> Gesendet: Mittwoch, 19. September 2007 11:13
> An: dev@httpd.apache.org
> Betreff: Re: [PATCH 43415] Logging remote port.
> 
> 
> Plüm wrote:
> 
> > 1. Please provide a patch against trunk.
> 
> mod_config_logger.c hasn't changed in trunk, so the patch 
> will work fine.

Sorry. Just a default comment if someone sends in a patch that is not
against trunk :-).

> 
> > 2. Please also add a patch for the documentation.
> 
> Done.

Fine, I have already seen it in the report.

> 
> > 3. I am not too happy with using %R, but to be honest I 
> have no better proposal :-).
> >    Maybe other have.
> 
> Well, then... ;)

I will leave around for just one or two days. If nobody has a better
idea we just take %R. Feel free to bug me if your patch falls off my radar.

> 
> > 4. 
> > 
> > Instead of using
> > +	return apr_psprintf(r->pool, "%u", 
> r->connection->remote_addr->port);
> > I would prefer
> > +	return  pfmt(r->pool, (int) (r->connection->remote_addr->port));
> > like used for log_status.
> 
> Well, in log_server_port, apr_psprintf is used, so that's 
> what I used. 
> But I really don't care one way or the other. Is there a particular 
> reason for using one or the other?

I think that using pfmt is more efficient and burns less cycles than apr_psprintf
(I think this would be also the case for log_server_port), but I may be wrong on this.
Additionally pfmt checks if port is <= 0 and logs a "-" in this case, but this should not
be the case for r->connection->remote_addr->port, so we could use format_integer
or
apr_itoa directly.

Regards

Rüdiger



Mime
View raw message