FreeTDS Development Group

["ZIGLIO Frediano" <Frediano.Ziglio AT vodafone.com>]

> 
> I want to make two changes to the sources, unless someone objects.  
> 
> 1.
> 
> We have many constructions like:
> 
>       if (cd != (iconv_t) - 1)
> 
> which is weird.  It looks like we're subtracting 1 from 
> iconv_t.  It should be:
> 
>       if (cd != (iconv_t)-1)
>       

Usually I use indent to fix style, I think indent is the problem... IMHO
anyone should indent(1) all code before commit... perhaps a perl script
to fix some strange formattings (like - 1 instead of -1) should be a
good solution ... I know a big perl guru :)

> but even that's not very good.  I say we need an entry in tdsiconv.h:
> 
>       static const iconv_t INVALID_DESCRIPTOR = (iconv_t)-1;
> 
> so we can say:
> 
>       if (cd != INVALID_DESCRIPTOR)
> 
> which is what we mean.  
> 

IMHO is better a #define for portability problems...

> 2.
> 
> Our comment blocks are inconsistently formatted.  I like BSD 
> "kernel normal form":
> 
>       /*
>        * Multiline comments are formatted like this, 
>        * and phrased as sentences.
>        */
> 
> If you haven't read Tufte's "The Visual Display of 
> Quantitative Information", you should; everyone should.  (It 
> sounds like a tome, I know, but it's actually a riff.)  One 
> of his principles is that, in diplaying information, the 
> data:nondata ratio should be kept as high as possible.  KNF 
> style makes comments obvious without extraneous elements. 
> It's also easier to modify than a "boxed" style.  
> 
> Further information on KNF:
> 
> http://www.netbsd.org/Documentation/kernel/programming.html#knf
> 

We already use a similar format... I don't understand the difference...

> +++
>
> I have one other change I think we should adopt:  We should use enums
more than we do, and lean on the preprocessor less.  
> For instance, to pick on a certain large patch recently submitted:
> 
> +#define TDS_SP_CURSOR          1
> +#define TDS_SP_CURSOROPEN      2
> +#define TDS_SP_CURSORPREPARE   3
> [...]
> 
> These are enumerated stored procedure ids, and are better constructed
as:
> 
> typedef enum _stored_procedure_id {
>         TDS_SP_CURSOR = 1
>       , TDS_SP_CURSOROPEN = 2
>       , TDS_SP_CURSORPREPARE = 3
>       [...]
> } stored_procedure_id;
>
> enums are easier (i.e., possible) for debuggers and compilers to vet
and decode.  There are times when we can't use them, usually because
they're equivalent to unsigned ints, and we need negative numbers or
chars or something.  But when we can use them, we're better off if we
do.  
> 
> Comments invited.

Agreed.

About ODBC formatting. We have attribute like henv, hdbc that are just
pointers, not handles. I'd like to remove "h" from name. Another are
attributes. There are a structure with all attribute with names starting
with "attr_". Perhaps is better to remove these attr structure and
incorporate attributes in _hdbc/_henv/_hstmt.

freddy77