apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: svn commit: r329089 - in /apr/apr-util/trunk: build.conf include/apr_memcache.h memcache/ memcache/apr_memcache.c
Date Fri, 28 Oct 2005 06:56:01 GMT
On Thu, Oct 27, 2005 at 11:22:42PM -0700, Paul Querna wrote:
> William A. Rowe, Jr. wrote:
> > pquerna@apache.org wrote:
> >> Author: pquerna
> >> Date: Thu Oct 27 21:23:01 2005
> >> New Revision: 329089
> >>
> >> URL: http://svn.apache.org/viewcvs?rev=329089&view=rev
> >> Log:
> >> Import apr_memcache trunk to APR-Util.
> >>
> >> Still TODO:
> >> - Move CRC32 to a separate public function/API.
> >> - Create a multi-get interface.
> >> - Make locking and connection pooling optional.
> > 
> > For the moment, -1, please revert and begin an apr-memcache sandbox
> > because...
> > 
> >  * it makes trunk/ unstable, preventing us from moving to 1.3
> 
> Then branch 1.3 and revert in the branch.  Trunk should always be safe
> to commit to, IMHO.

Agreed.  Some review:

- apr_memcache_hash's data_len argument, apr_memcache_conn_t.blen , 
storage_cmd_write's *_size  arguments are all apr_uint32_t and look like 
they should be apr_size_t, any reason why that is?

similarly "int key_size = strlen(key);" -> apr_size_t

- code style nits, "if(" -> "if (", lots of indenting weirdness, some 
"char* foo"

- use apr_snprintf not snprintf

- I suppose apr_brigade_split() use should be "considered harmful" due 
to memory consumption ;)

joe

Mime
View raw message