On Tue, Mar 30, 2004 at 07:05:22AM -0500, Jeff Trawick wrote:
> [EMAIL PROTECTED] wrote:
> >jorton 2004/03/30 01:58:22
> >
> > Modified: xlate xlate.c
> > Log:
> > * xlate/xlate.c (check_sbcs): Remove function which made unsafe
> > assumptions (a theoretical issue), and was buggy in not resetting the
> > iconv handle on failure (the cause of at least one real issue).
> > (apr_xlate_open): Updated caller.
>
> there is some additional work to be done with this... I have a patch to
> yank apr_xlate_sb_get() and modify the various apr-util callers of it, but
> probably won't get it committed until tonight or tomorrow a.m....
Yeah, I was going to commit the more radical changes separately...
attached what I have.
> apr-util 0.9 will mark the apr_xlate_sb_get() API deprecated, and of course
> your fix to reset the iconv handle on failure needs to be in 0.9
Actually I thought that the simple removal of check_sbcs() would be
appropriate for 0.9 too, rather than wait for someone to find a case
where it "does the wrong thing".
> additionally, the single-byte conversion routines need to be reworked to
> not depend on the sbcs table, which now is purely an identity table which
> is used when the to and from charsets are the same
With just identity tables remaining, conv_byte is either a nullop or an
error, so I removed it. But you did want to keep it around for a future
where APR has hard-coded single-byte-lookup tables for some
translations?
> but then the identity table use, which was implemented to piggyback on the
> sbcs "optimization," should be replaced with memcpy() ;)
Indeed, makes it much simpler :)
joe
Index: xlate/xlate.c
===================================================================
RCS file: /home/cvs/apr-util/xlate/xlate.c,v
retrieving revision 1.20
diff -u -r1.20 xlate.c
--- xlate/xlate.c 30 Mar 2004 09:58:22 -0000 1.20
+++ xlate/xlate.c 30 Mar 2004 13:08:00 -0000
@@ -54,9 +54,7 @@
struct apr_xlate_t {
apr_pool_t *pool;
- char *frompage;
- char *topage;
- char *sbcs_table;
+ int identity;
#if APU_HAVE_ICONV
iconv_t ich;
#elif APU_HAVE_APR_ICONV
@@ -101,23 +99,12 @@
return APR_SUCCESS;
}
-static void make_identity_table(apr_xlate_t *convset)
-{
- int i;
-
- convset->sbcs_table = apr_palloc(convset->pool, 256);
- for (i = 0; i < 256; i++)
- convset->sbcs_table[i] = i;
-}
-
APU_DECLARE(apr_status_t) apr_xlate_open(apr_xlate_t **convset,
const char *topage,
const char *frompage,
apr_pool_t *pool)
{
- apr_status_t rv;
apr_xlate_t *new;
- int found = 0;
*convset = NULL;
@@ -125,71 +112,43 @@
frompage = handle_special_names(frompage, pool);
new = (apr_xlate_t *)apr_pcalloc(pool, sizeof(apr_xlate_t));
- if (!new) {
- return APR_ENOMEM;
- }
-
new->pool = pool;
- new->topage = apr_pstrdup(pool, topage);
- new->frompage = apr_pstrdup(pool, frompage);
- if (!new->topage || !new->frompage) {
- return APR_ENOMEM;
- }
#ifdef TODO
/* search cache of codepage pairs; we may be able to avoid the
- * expensive iconv_open()
+ * expensive iconv_open().
*/
-
- set found to non-zero if found in the cache
#endif
-
- if ((! found) && (strcmp(topage, frompage) == 0)) {
+
+ if (strcmp(topage, frompage) == 0) {
/* to and from are the same */
- found = 1;
- make_identity_table(new);
+ new->identity = 1;
}
-
+ else {
#if APU_HAVE_APR_ICONV
- if (!found) {
rv = apr_iconv_open(topage, frompage, pool, &new->ich);
if (rv != APR_SUCCESS) {
return rv;
}
- found = 1;
- } else
- new->ich = (apr_iconv_t)-1;
-
#elif APU_HAVE_ICONV
- if (!found) {
new->ich = iconv_open(topage, frompage);
if (new->ich == (iconv_t)-1) {
int rv = errno;
/* Sometimes, iconv is not good about setting errno. */
return rv ? rv : APR_EINVAL;
}
- found = 1;
- } else
- new->ich = (iconv_t)-1;
#endif /* APU_HAVE_ICONV */
-
- if (found) {
- *convset = new;
- apr_pool_cleanup_register(pool, (void *)new, apr_xlate_cleanup,
- apr_pool_cleanup_null);
- rv = APR_SUCCESS;
+ apr_pool_cleanup_register(pool, new, apr_xlate_cleanup,
+ apr_pool_cleanup_null);
}
- else {
- rv = APR_EINVAL; /* iconv() would return EINVAL if it
- couldn't handle the pair */
- }
-
- return rv;
+
+ *convset = new;
+ return APR_SUCCESS;
}
APU_DECLARE(apr_status_t) apr_xlate_sb_get(apr_xlate_t *convset, int *onoff)
{
- *onoff = convset->sbcs_table != NULL;
+ *onoff = convset->identity;
return APR_SUCCESS;
}
@@ -201,8 +160,18 @@
{
apr_status_t status = APR_SUCCESS;
+ if (convset->identity) {
+ apr_size_t len = *inbytes_left;
+
+ if (len > *outbytes_left) len = *outbytes_left;
+
+ memcpy(outbuf, inbuf, len);
+
+ *outbytes_left -= len;
+ *inbytes_left -= len;
+ }
+ else {
#if APU_HAVE_APR_ICONV
- if (convset->ich != (apr_iconv_t)-1) {
const char *inbufptr = inbuf;
apr_size_t translated;
char *outbufptr = outbuf;
@@ -245,11 +214,7 @@
default:
break;
}
- }
- else
-
#elif APU_HAVE_ICONV
- if (convset->ich != (iconv_t)-1) {
const char *inbufptr = inbuf;
char *outbufptr = outbuf;
apr_size_t translated;
@@ -295,42 +260,18 @@
break;
}
}
- }
- else
#endif
-
- {
- int to_convert = min(*inbytes_left, *outbytes_left);
- int converted = to_convert;
- char *table = convset->sbcs_table;
-
- while (to_convert) {
- *outbuf = table[(unsigned char)*inbuf];
- ++outbuf;
- ++inbuf;
- --to_convert;
- }
- *inbytes_left -= converted;
- *outbytes_left -= converted;
}
return status;
}
-APU_DECLARE(apr_int32_t) apr_xlate_conv_byte(apr_xlate_t *convset,
- unsigned char inchar)
-{
- if (convset->sbcs_table) {
- return convset->sbcs_table[inchar];
- }
- else {
- return -1;
- }
-}
-
APU_DECLARE(apr_status_t) apr_xlate_close(apr_xlate_t *convset)
{
- return apr_pool_cleanup_run(convset->pool, convset, apr_xlate_cleanup);
+ if (convset->identity)
+ return APR_SUCCESS;
+ else
+ return apr_pool_cleanup_run(convset->pool, convset, apr_xlate_cleanup);
}
#else /* !APR_HAS_XLATE */
@@ -346,12 +287,6 @@
APU_DECLARE(apr_status_t) apr_xlate_sb_get(apr_xlate_t *convset, int *onoff)
{
return APR_ENOTIMPL;
-}
-
-APU_DECLARE(apr_int32_t) apr_xlate_conv_byte(apr_xlate_t *convset,
- unsigned char inchar)
-{
- return (-1);
}
APU_DECLARE(apr_status_t) apr_xlate_conv_buffer(apr_xlate_t *convset,
Index: include/apr_xlate.h
===================================================================
RCS file: /home/cvs/apr-util/include/apr_xlate.h,v
retrieving revision 1.9
diff -u -r1.9 apr_xlate.h
--- include/apr_xlate.h 13 Feb 2004 09:55:26 -0000 1.9
+++ include/apr_xlate.h 30 Mar 2004 13:08:00 -0000
@@ -127,17 +127,6 @@
#endif
/**
- * Convert a single-byte character from one charset to another.
- * @param convset The handle allocated by apr_xlate_open, specifying the
- * parameters of conversion
- * @param inchar The single-byte character to convert.
- * @warning This only works when converting between single-byte character sets.
- * -1 will be returned if the conversion can't be performed.
- */
-APU_DECLARE(apr_int32_t) apr_xlate_conv_byte(apr_xlate_t *convset,
- unsigned char inchar);
-
-/**
* Close a codepage translation handle.
* @param convset The codepage translation handle to close
* @remark
