[freetds] new dstr changes

ZIGLIO, Frediano, VF-IT Frediano.Ziglio at vodafone.com
Wed Aug 30 19:37:29 EDT 2006


> 
> On Wed, Aug 30, 2006 at 03:55:06PM +0200, ZIGLIO, Frediano, 
> VF-IT wrote:
> > > 
> > > On Tue, Aug 29, 2006 at 05:26:36PM +0200, ZIGLIO, Frediano, 
> > > VF-IT wrote:
> > > > I also noted that in code there is some code to use a 
> > > static allocated
> > > > buffer if possible (bcp if I reminds correctly), perhaps 
> > > something like
> > > 
> > > I think we sometimes use  DSTR structures where static 
> allocations or 
> > > ordinary char* would suffice.  I can't think of any 
> reason to use a 
> > > "statically allocated" DSTR.  
> > 
> > I think we agree to add a length field. 
> 
> I want to say clearly: I don't understand why we use DSTR.  I think it
> makes the code opaque.  I don't see the advantage.  In 
> particular, I think
> it's made the construction of a login packet harder.  
> 

currently the only difference (beside syntax) between a char * and a
DSTR is that DSTR is never NULL... So, currently there are very few
advantages (you can't init with a simple memset, but it can't core if
someone try to do a strlen(DSTR)). I mainly wrote DSTR stuff with the
idea to extend it a day or the other. One of the reasons was length for
encodings with NUL embedded, another was buffer handling. I think that
problem with login is that in many place we still use a char* with a
length (or even without length) so we have to convert.

> However, you've thought about this, and you're looking to improve
> DSTR.  If you want my opinion, I think less indirection is better, 
> regardless of difficulty of initialization.  So I guess I like 
> option #1.  Except that I'd put the length first, the way TDS 
> does, the
> way Pascal does.  Then it just becomes a counted string.  That's more 
> robust; it's harder to overwrite your length with too-long data.  
> 

No problem, I put the pointer before length cause so it could be better
aligned on platforms where size_t is 32bit and char* 64bit (like
Windows), sometime I think in assembly :) . I also don't see many
advantages to use size_t instead of unsigned int (usually smaller for
sufficient for our strings). Note that data can't overwrite structure in
this case (data is kept at different location, the problem arise with
structure like 2).
I agree that if we have to choose 1 is the better trade-off.

> If you're going to add a length, maybe call it 'size', instead?  And 
> you might want to consider keeping a capacity:
> 
> struct DSTR_STRUCT { 
>         size_t capacity, size;
> 	char *data;
> };
> typedef struct DSTR_STRUCT DSTR;
> 
> Regards, 
> 

You speak english more than me, if size is better for you size is
better.

I thinked about a capacity and I have nothing against it, only it seems
we don't need that much.
Well... here is late, I'm currently at work (it's 01:30 AM here!),
someone is doing some test at the electrical system so I have to stay
here just in case something go wrong (that is if all is fine I have
nothing to do). Perhaps we could use capacity == 0 (and size >= 0) to
indicate preallocated?? No... we can use external buffers (constant) but
no static allocated one (see TDS_PBCP structure in src/dblib/bcp.c) ... 

A note about binary compatibility. DSTR is used only internally and by
libTDS so only libTDS is affected. Currently CVS HEAD do not produce a
dynamic library for libTDS (as stated in previous mail). This is a big
change but I don't think it will break compatibility so much (every
release we changed libTDS compatibility just after some weeks after
previous release...) but I think it's the only way to not break binary
compatibility in future release.

Regards
  Frediano

PS: ZzzzZZzzz...




More information about the FreeTDS mailing list