Skip to Content.
Sympa Menu

freetds - [freetds] replacements/iconv.c questions

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: jklowden AT schemamania.org
  • To: FreeTDS Development Group <freetds AT lists.ibiblio.org>
  • Subject: [freetds] replacements/iconv.c questions
  • Date: Mon, 31 Jan 2011 17:17:02 -0500

Hi Freddy,

I have some questions about how tds_sys_iconv() is written. I'm looking at it
because VS 2005 kicks out warnings for a lot of stuff related to iconv_t, and
before re-writing it, I'd like to understand why certain choices were made:

1. Why do all the get/put functions e.g. put_utf16le() use int instead of
size_t? inbytesleft and outbytesleft are size_t*, and il and ol are size_t,
but
all the little functions take int as their length argument and many check for
a
negative length.

2. This comment:

* also we use unsigned to remove required unsigned casts

Does that refer to warnings we would get in the get/put functions if they
took signed characters instead of unsigned?

3. Why the function pointer arrays iconv_gets and iconv_puts? Why not a
simple
switch statement instead? The array is referenced only by tds_sys_iconv() and
a switch would be a lot clearer than bit-twiddling the iconv_t value.

I could understand using function pointers if tds_sys_iconv_open() *set* the
pointers and tds_sys_iconv() *used* them. As it is, though,
tds_sys_iconv_open()
hacks two offsets into the iconv_t, and tds_sys_iconv() unpacks them to look
up
the functions.

Suppose we did this instead:

In tds_sys_iconv_open():

struct get_put_pair {
iconv_get_t get;
iconv_put_t put;
};

get_put_pair *cd = calloc(1, sizeof(get_put_pair));

/* look up names, set members */

/* ... e.g. ... */

if (strcmp(enc_name, "US-ASCII") == 0)
cd->get = get_ascii;

/* ... */

return cd;

In tds_sys_iconv():

iconv_get_t get_func = ((struct get_put_pair*)cd)->get;
iconv_put_t put_func = ((struct get_put_pair*)cd)->put;


In tds_sys_iconv_close():

free(cd);

That would be much more portable and understandable code, don't you agree?

I don't even see why we would bother to define iconv_t as void*. We define it
only for FreeTDS in tdsiconv.h. It could just as easily be a get_put_pair*.

Here's to hoping SF is back online soon.

Regards,

--jkl




  • [freetds] replacements/iconv.c questions, jklowden, 01/31/2011

Archive powered by MHonArc 2.6.24.

Top of Page