FreeTDS Development Group

[arielCo <arielco AT gmail.com>]

* I agree wholeheartedly, with a minor change. A function returning an
int would be better called something like tds_convert_size, to give a
user/reader the right idea about the return value; and of course the
user would test for convertibility like "if (bufsize =
tds_convert_size(...))" or "if (colwidth = PRINTABLE_SIZE(...))".

* This function needs to accept the source column size, so that the
PRINTABLE_SIZE macro produces meaningful info for variable-width data
types like SYBCHAR. Then, PRINTABLE_SIZE(type, size) would call
tds_convert_size(type, size, SYBCHAR).

* Now, I mentioned that a recent commit introduced the boolean-ization
that you rightly despise, and it broke my original fix:

http://gitorious.org/freetds/freetds/commit/1bb190f304604bd7338c5df9bddf81af7e7a9ad9
dbwillconvert(int srctype, int desttype)
{
        tdsdump_log(TDS_DBG_FUNC, "dbwillconvert(%s, %s)\n",
tds_prdatatype(srctype), tds_prdatatype(desttype));
-       return tds_willconvert(srctype, desttype);
+       return tds_willconvert(srctype, desttype) ? TRUE : FALSE;
}

It addresses "Bug #58 reported by Mikhail Teterin", but I don't even
know where the tracker is and I have to see that I keep or replicate
Frediano's fix. Of course, I have grep the whole tree for calls to
*_willconvert and get_printable_size anyway, so I'll check how the
return values are used currently.

>From a compulsive "fixer", glad to help. I'll get to it over the next week.

Ariel Cornejo
+58·412·8083546


On Wed, Jun 26, 2013 at 7:47 PM, James K. Lowden <jklowden AT freetds.org> wrote:
> On Fri, 21 Jun 2013 18:14:10 -0430
> arielCo <arielco AT gmail.com> wrote:
>
>> James, I substituted dbwillconvert() successfully in the 0.91 tarball,
>> but in Git it now boolean-izes the result of tds_willconvert:
>>       return tds_willconvert(srctype, desttype) ? TRUE : FALSE;
>> Which makes more sense given the function name, but dbwillconvert
>> remains a scalar.
>>
>> Since there are three mostly-redundant switch statements, and the one
>> in tds_willconvert is almost complete, maybe:
>> * have a single switch statement in, say, get_printable_size
>> (type,size)
>> * _get_printable_size(colinfo) would be a wrapper
>> * tds_willconvert(srctype, desttype) calls get_printable_size(),
>> perhaps mapping SYBINTN to SYBINT8 since the size doesn't matter
>> * dbwillconvert stays a wrapper
>> * both *willconvert() functions return a boolean
>
> Hi Ariel,
>
> Thanks for working on this, first of all, on behalf of the community.
>
> My preference would be:
>
> 1.  put all the work in tds_willconvert()
> 2.  #define PRINTABLE_SIZE(x, SYBCHAR)
> 3.  remove _get_printable_size() and get_printable_size
> 4.  return int
>
> because
>
> 1.  get_printable_size() could otherwise return a
> value for an input type that tds_convert() cannot handle (or that
> tds_willconvert() says cannot be handled).  This way, an invalid input
> is uniformly and correctly handled.
>
> 2.  This is C, and the rule in C is that 0 is false and anything else
> is true.  Reducing the return code from "size of output" to "yes or no"
> removes information to no advantage.  (I have no sympathy for the C
> programmer who writes
>
>         if (dbwillconvert(SYBINT, SYBCHAR) == TRUE)
>
> because, well, there oughta be a law.)
>
> 3.  No self-respecting C function, except I/O, should begin with "get".
> It's a *function*!  It maps input to output.  Observe:
>
>         int len = get_printable_size(SYBINT);
>         int len = printable_size(SYBINT);
>
> The "get" is just nonce noise.  In the same way we don't say
>
>         double x = compute_square_root(y);
> or
>         double x = get_sqrt(y);
> but
>         double x = sqrt(y);
>
> Save typing.  Read faster.  Go home early.  ;-)
>
> Hope you're convinced, and thanks again for putting in the time.
>
> --jkl
>
>
>
>> On Fri, Jun 14, 2013 at 10:00 PM, arielCo <arielco AT gmail.com> wrote:
>> > On Thu, Jun 13, 2013 at 6:06 PM, James K. Lowden
>> > <jklowden AT freetds.org> wrote:
>> >> The right thing to do -- your mission, should you choose to accept
>> >> it
>> >> -- is to use tdswillconvert() instead to determine the size.
>> > Challenge accepted. I'll keep you posted.
>> >
>> >
>> > Ariel Cornejo
>> > +58·412·8083546
>> > +58·416·6189113
>> _______________________________________________
>> FreeTDS mailing list
>> FreeTDS AT lists.ibiblio.org
>> http://lists.ibiblio.org/mailman/listinfo/freetds
> _______________________________________________
> FreeTDS mailing list
> FreeTDS AT lists.ibiblio.org
> http://lists.ibiblio.org/mailman/listinfo/freetds