Re: tdsver patch

On Tue, Aug 25, 2009 at 11:02:37AM +0200, Frediano Ziglio wrote:
> Hi,
>   just yesterday I decided to wrote a patch for tds version using MS
> specifications (that is 7.1 for mssql 2000 and 7.2 for mssql 2005).
> The patch is large but mainly a "sed" patch. I also merged
> major_version and minor_version fields in a new tds_version field and
> updated userguide.
> Please check the patch.

Hi Freddy, 

Mostly looks good, nice clean-up.  

We might not really need these anymore:

+#define IS_TDS7_PLUS(x) ((x)->tds_version>=0x700)
+#define IS_TDS71_PLUS(x) ((x)->tds_version>=0x701)

because this code could be made clearer:

+               if (IS_TDS71_PLUS(tds))

as just 

+               if (tds->tds_version >= 0x701)

The old _PLUS macro hid a logical '&&' test that is gone now that the major and minor versions are combined.  

But it's also OK to keep, if you prefer.  Maybe better for grepping.  

+       connection->tds_version = (TDS_DEF_MAJOR << 8) + TDS_DEF_MINOR;

I would replace  TDS_DEF_MAJOR and  TDS_DEF_MINOR with TDS_DEFAULT_VERSION.  

There are also references to "8.0" in doc/*.txt.  

Thanks for doing it, please apply.  

Regards, 

--jkl