Skip to Content.
Sympa Menu

freetds - RE: [freetds] errno and threads

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: Frediano Ziglio <freddyz77 AT tin.it>
  • To: FreeTDS Development Group <freetds AT lists.ibiblio.org>
  • Subject: RE: [freetds] errno and threads
  • Date: Tue, 05 Aug 2003 19:26:28 -0000

Il mar, 2003-08-05 alle 19:06, Lowden, James K ha scritto:
> > From: ZIGLIO Frediano [mailto:Frediano.Ziglio AT vodafone.com]
> > Sent: August 5, 2003 10:42 AM
> > >
> > > > Another issue about iconv. I'm going to change iconv
> > > > stuff using an abstract interface...
> > >
> > > You mean something along the lines of funopen(3), with I/O
> > > function pointers instead of buffers? That might be OK. I
> > > looked at it, but I didn't see much leverage. Everything
> > > ultimately comes down to translating buffers, and not all the
> > > data are subject to conversion. Something has to interpret
> > > the data stream and separate the character data from nonchar
> > > data, so there's no notion of plugging iconv() into a socket.
> > >
> > > Could you give me a clearer idea of what you have in mind?
> > >
> >
> > What I aim most is to separate iconv code. tdsiconv.h and
> > iconv_t system type should be used onlt by iconv.c.
>
> Freddy, I think we might be working at cross purposes. I don't see any
> value in isolating iconv_t or the effects of iconv(). On the contrary, I
> think we should embrace it.
>

Well:
- win32 don't have iconv
- Mac OS X don't have iconv
- older unix do not have iconv
so why embrace iconv ??

> I see value in tds_iconv() primarily to localize the selection of iconv_t
> handles, and as a place to generate client-side messages.
>
> One way to deal with character set conversions is the "old" way:
>
> 1. Determine if conversion is necessary.
> 2. Determine if conversion is possible (HAVE_ICONV).
> 3. Branch accordingly.
>

However you cannot include tdsiconv.h in tds.h cause tdsiconv.h need
HAVE_ICONV that need config.h. It's very bad including config.h (or have
to add -DHAVE_ICONV to all programs compiled with FreeTDS), so it's bad
to include tdsiconv.h in tds.h (cause libraries need to install tds.h).

> This sprinkles "if unicode" and "if have iconv" all over the code.
>
> My way, the "new" way (so far):
>
> 1. Convert everything.
> 2. (Let the converter determine what's feasible/necessary.)
>

?? I don't follow you ...

> I wrote replacements/iconv.c especially so we wouldn't have to decide how to
> treat each and every character buffer. Just convert it. If no conversion
> was actually necessary, iconv() will copy it. Yes, that means extra buffer
> copies for like-to-like "conversions", but it makes the code smaller and
> simpler over all.
>
> > (I found also an old mail with same hints...)
> > Mainly we need some functions
> > - initialization
> > - free
> > - getting a conversion (should return just a undefined pointer)
> > how to retrieve/cache server/default conversions ??
> > - convert (using previous pointer)
>
> We have all this.
>

Yes, but initialization free is spread between iconv.c and mem.c. Using
an abstract layer we can use #ifdef HAVE_ICONV in a single file
(iconv.c). We have also 2 convert function (one for buffer and one for
file) and function that need to convert a long buffer are very hard (you
can see from all fix we wrote).

> > About funopen... I don't know... I have to resolve some
> > problems before
> > starting coding
> > 1- how to get ratio transformation (token.c)
> > 2- avoid iconv_t use (bcp.c)
> > 2 suggest my an idea similart to funopen. Only slightly different
> > structures
> > typedef struct tds_read
> > {
> > /** pointer to position to read */
> > TDS_CHAR *ptr;
> > /** pointer to buffer end */
> > TDS_CHAR *end;
> > /** fill again buffer */
> > int (*read)(struct tds_read* rd);
> > } TDSREAD;
> >
> > typedef struct tds_write
> > {
> > /** pointer to position to write */
> > TDS_CHAR *ptr;
> > /** pointer to buffer end */
> > TDS_CHAR *end;
> > /** flush buffer */
> > int (*write)(struct tds_read* rd);
> > } TDSWRITE;
> >
> > int tds_conv(TDSSOCKET*, TDSICONVINFO*, TDSREAD*, TDSWRITE*);
> > direction, others ??
>
> Well, OK. You're really just passing two more pointers (the read/write
> functions) and using *end instead of inbytesleft (or outbytesleft). You
> will have to set up those structures. Then, eventually you'll have to talk
> to iconv(3). When you do, you'll have to:
>
> 1. Unpack the structures.
> 2. Calculate inbytesleft and outbytesleft.

Just 2 subtraction...

> 3. Call iconv().
> 4. Update tds_write::end and tds_read::end
>
> Are you sure you want to do this?
>

Well. TDSREAD and TDSWRITE are abstract. I take the idea from C++
iostream implementation. TDSWRITE can be a writer for packet so instead
of converting to a buffer and then copy to packet buffer and send to
network you can copy directly to packet buffer and flush. TDSWRITE can
be a static buffer so TDSWRITE::write do nothing or it can be a dynamic
buffer so TDSWRITE::write can extend buffer (still a FIXME in
tds_get_data...). Same for TDSREAD...

> I think you're setting up a layer of abstraction that is opaque, making the
> code less accessible. Anyone interested in this issue will know or can
> learn about iconv. By placing our abstraction over it, we risk making it
> harder to understand. I wouldn't mind that especially, except that I still
> see no gain. I just don't think you're going to achieve any code reduction.
>

In future version (0.63 ??) I think to do a large use of such data
(TDSREAD and TDSWRITE), mainly to remove memory problems converting
data...

>
> I think what you're trying to avoid is the loop that several functions use
> to call tds_iconv. I think there are few such loops, all slightly
> different. Most calls to tds_iconv do/will use small fix-length buffers and
> so won't require a loop.
>

We use also a lot of dynamic strings (sql statement, text, xvarchar)...

> > (why iconv ? IMHO conv it's a more portable name)
>
> I think it's important to distiguish between datatype conversion
> (tds_convert) and character set conversion (tds_iconv). "tds_conv" doesn't
> suggest either one clearly.
>

You are right. However iconv is too similar to... iconv :)

bye
freddy77






Archive powered by MHonArc 2.6.24.

Top of Page