apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Showstopper ... was: Tagged the tree
Date Wed, 08 Jan 2003 15:15:56 GMT
At 07:00 PM 1/7/2003, you wrote:
>Sander Striker wrote:
>
>>Hi,
>>
>>I tagged the tree with STRIKER_2_0_44_PRE2.  The tag consists
>>of APACHE_2_0_BRANCH and apr/apr-util HEAD.  If you feel that
>>something should not be in here, please let me know ASAP.
>
>What about the change in argument types for the APR queue
>and hash API?  That's the one thing I know of that would
>break binary compatibility.

Beyond the queue and hash changes that break on all 64 bit platforms 
(and should be reverted and deferred to APR 1.0), here are some other 
interesting bits...

 APR_DECLARE(apr_status_t) apr_generate_random_bytes(unsigned char * buf,
-                                                    int length);
+                                                    apr_size_t length);

won't pose a problem except on 64 bit LP and P platforms... on those
few (Win64/AIX/dunno which others) it should be postponed for the 
APR 1.0 release (targeted by httpd-2.2.)

Just an observation reviewing the apr/includes/ changes... I don't like the
look of this code;

+#define apr_atomic_casptr(mem,with,cmp) (void*)atomic_cmpxchg((unsigned long *)(mem),(unsigned
long)(cmp),(unsigned long)(with))

Very simply, this is a very easily broken macro... it is too easy to silently 
pass the wrong pointer arg to apr_atomic_casptr and explode.

We need some type safety here, and this macro (I believe) destroys
all hope of that.  It seems this needs to become a function.  Macro to
function should be a safe change, since code compiled to an earlier
apr will continue to use this unsafe but otherwise usable macro.

For something completely different, once this is released, we are stuck
with the api...

#define APR_FILEPATH_ENCODING_UNKNOWN  0
#define APR_FILEPATH_ENCODING_LOCALE   1
#define APR_FILEPATH_ENCODING_UTF8     2
APR_DECLARE(apr_status_t) apr_filepath_encoding(int *style, apr_pool_t *p);

Why not drop the enum and use an apr_filepath_encoding that returns
an actual codepage name?  Then the ambiguity of _LOCALE disappears.

Put another way, either APR_FILEPATH_ENCODING_LOCALE
with a locale of utf-8 or the APR_FILEPATH_ENCODING_UTF8 can
mean the same thing.  This seems wrong.  For that matter, it might
also be APR_FILEPATH_UNKNOWN if we didn't determine it.

Just asking for such things to be as clean as we can ask before we roll.

Finally, Brane has a patch that completes our apr-iconv work (even for 
installing apr-iconv .so's when we aren't building httpd.)  I'm vetting it and
should have it committed today so we can call it baked on win32.

To the dev@apr members, anyone feel like calling this 0.9.2 in conjunction
with httpd's next release and pushing on to 0.9.3-dev???

Bill



Mime
View raw message