[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH]: Trying to get rid of iconv dependency on windows

From: Garrett Rooney <rooneg_at_electricjellyfish.net>
Date: 2005-10-12 21:50:51 CEST

One comment that occurred to me as I read over this patch...

> +typedef struct win_xlate_t {
> + apr_pool_t *pool;
> + char *frompage;
> + char *topage;
> + char *sbcs_table;
> +} win_xlate_t;

Storing a pool in this structure throws up a red flag for me...

> +static apr_status_t win_xlate_conv_buffer(win_xlate_t *convset,
> + const char * inbuf,
> + apr_size_t *inbytes_left,
> + char *outbuf,
> + apr_size_t *outbytes_left)
> +{
> + svn_boolean_t fromUTF8 = FALSE;
> + int requiredlen = 0;
> + wchar_t * widebuffer = 0;
> + apr_size_t convertedchars = 0;
> + UINT CP = CP_ACP;
> +
> + if (convset->frompage)
> + {
> + if (apr_strnatcmp (convset->frompage, "UTF-8") == 0)
> + fromUTF8 = TRUE;
> + else
> + CP = atoi(convset->frompage+2);
> + }
> + if ((fromUTF8)&&(convset->topage))
> + {
> + CP = atoi(convset->topage+2);
> + }
> + if (CP == 0)
> + CP = CP_THREAD_ACP;
> +
> + requiredlen = MultiByteToWideChar(fromUTF8 ? CP_UTF8 : CP, 0, inbuf, -1, 0, 0);
> + if (requiredlen == 0)
> + return APR_EINVAL;
> + widebuffer = apr_pcalloc(convset->pool, requiredlen*sizeof(wchar_t));
> + if (widebuffer == 0)
> + return APR_ENOMEM;

And this is why. Won't we be allocating that buffer every time we
call this function, without ever clearing that pool? If we really
need to allocate that buffer each time, we also need to be sure we're
clearing that pool when we're done with it, which means that
convset->pool really should be a specially created subpool just for
that purpose.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Oct 12 21:51:50 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.