FreeTDS Development Group

[Frediano Ziglio <freddy77 AT gmail.com>]

Il giorno ven, 11/01/2008 alle 13.34 -0500, James K. Lowden ha scritto:
> Sebastien FLAESCH wrote:
> > I started to look at it and it appears that many parameters are missing
> > in the ODBC config, so far I have added these (using src/tds/config.c as
> > ref):
> > 
> 
> These we need:
> 
> >    ClientCharset
> >    ConnectionTimeout
> >    DebugFlags
> >    Encryption
> >    QueryTimeout
> 

QueryTimeout and ConnectionTimeout can be set using standard ODBC calls
(SQLSetConnectAttr/SQLSetStmtAttr) so I would avoid.

DebugFlags and DumpFile are very bounded if you add one I would add the
other.

> These are IMO optional:
> 
> >    DumpFile
>       can use TDSDUMP instead
> >    Instance
>       can use instance\server instead
> 

The standard way (call it Microsoft way) to set instance is server
\instance. There is also a server,port syntax.

> These we don't need:
> 
> >    EmulateLittleEndian
>       Applies only to TDS 4.2
> >    ServerCharset
>       Not a client setting, server tells us
> >    SwapBrokenDates
> >    SwapBrokenMoney
>       Done automatically for many years
> 

I agree to not add these.

> FYI regarding charsets.  It should be the case that the client charset is
> first derived from the locale(1) environment.  Setting it in freetds.conf
> or odbc.ini overrides that default.  
> 
> > Started to hack and now I realize that it would be smarter to have the
> > src/odbc/connectparams.c module USING the src/tds/config.c module
> 
> Absolutely! 
> 

If you want to use src/tds/config.c to read odbc.ini entries I'm again
this. The reason is that different DMs could read this file in slightly
different way. Consider this
- different file encoding
- different quoting
- different file locations (/etc/odbc.ini or others)
- different override (for instance with unixODBC you can override
odbc.ini locations using environment)

> > I could also see that the ODBC code repeats the same string constants
> > several times => candidate for a #define in some header file...
> 
> Yes, or 
> 
>       static const char something[] = "something";
> 

Yes, it would perhaps be better to have a unique function or an array
like

struct option
{
  const char *name;
  unsigned int in_ini:1, in_string:1; /* boolean, where available */
  int (*handle_me)(...);
}

or similar...

> > I mean, I can do a dirty/small patch or a large/smart patch ...
> > 
> > Maybe I am too new to this project to make large changes, so I better
> > let you guys make it nicer once posted...
> 
> >From your participation so far, I'm optimistic.  Do what you think is
> right.  I would rather you do your best work and use your highest
> judgement.  The code is not sacred.  
> 
> If there's some issue with your patch, we can discuss it.  Normally
> technical questions sort themselves out because they have more-or-less
> obvious best answers.  
> 
> > Also, are there any test cases I should add / adapt (I could not find
> > anything in src/odbc/unittests reading an odbc.ini file)
> 
> That would be good if you can do it, particularly if all libraries use
> tds/config.c.  I find that the existence of a unit test increases my
> ability/confidence to make a change.  I can only imagine others feel the
> same way.  
> 
> Regards, 
> 
> --jkl

bye
  freddy77