FreeTDS Development Group

["ZIGLIO, Frediano, VF-IT" <Frediano.Ziglio AT vodafone.com>]

> 
> > From: Frediano, VF-IT
> > Sent: Monday, July 10, 2006 11:19 AM
> > 
> >   trying to fix rpc2.php test (and some done_handling) I came to
> >   this
> > old patch. This is pre 0.63 !!!
> 
> You've been working....
> 
> >
>
https://sourceforge.net/tracker/index.php?func=detail&aid=1095806&group_
id=33106&atid=407808 Do you remember the print problem?? 
> 
> I think so.  IIRC, SQL PRINT statement output was getting ignored
> instead of being routed to the message handler.  
> 

yes, but I think that dbutil.c change is sufficient to fix the problem.
Patch for rpc.c instead fix dbrpcparam (point 3) so other change is
related to dbsqlok eating too many tokens... I think that we discover
that dblib have to eat more tokens...

> > 
> > @@ -1304,6 +1308,10 @@
> >             dbproc->dbresults_state = _DB_RES_INIT;
> >             return NO_MORE_RESULTS;
> >             break;
> > +   case _DB_RES_NEXT_RESULT:
> > +   case _DB_RES_INIT:
> > +           tds_free_all_results(tds);
> > +           break;
> >     default:
> >             break;
> >     }
> 
> Well, I suspect at the time libtds was less careful to free 
> its results.
> Over time we've moved more responsibility into libtds.  
> 
> On the same topic, src/dblib/unittests/rpc.c is now coded correctly
> (gets parameters & return status after dbresults() returns
> NO_MORE_RESULTS) and works!  As you can see, several days of code
> inspection yielded a two-line change....
> 

I'll try with ms dblib.

> I still plan to remove dbresults_state, but it's hard.  I think I'll
> have to hack on token.c first, and build better functions to 
> read packet
> sequences.  In case you're curious, here's a cross-reference:  
> 
> 0 dblib.h <global>     130 int dbresults_state;
> 1 dblib.c _dbresults  1309 dbproc->dbresults_state,
prdbresults_state(dbproc->dbresults_state));
> 2 dblib.c _dbresults  1310 switch ( dbproc->dbresults_state ) {
> 3 dblib.c _dbresults  1313 dbproc->dbresults_state =
_DB_RES_NEXT_RESULT;
> 4 dblib.c _dbresults  1342 dbproc->dbresults_state =
_DB_RES_RESULTSET_EMPTY;
> 5 dblib.c _dbresults  1351 dbproc->dbresults_state =
_DB_RES_RESULTSET_ROWS;
> 6 dblib.c _dbresults  1358 dbproc->dbresults_state,
prdbresults_state(dbproc->dbresults_state));
> 7 dblib.c _dbresults  1366 switch (dbproc->dbresults_state) {
> 8 dblib.c _dbresults  1370 dbproc->dbresults_state =
_DB_RES_NEXT_RESULT;
> 9 dblib.c _dbresults  1377 dbproc->dbresults_state =
_DB_RES_NEXT_RESULT;
> a dblib.c _dbresults  1391 switch (dbproc->dbresults_state) {
> b dblib.c _dbresults  1394 dbproc->dbresults_state =
_DB_RES_NEXT_RESULT;
> c dblib.c _dbresults  1398 dbproc->dbresults_state =
_DB_RES_NEXT_RESULT;
> d dblib.c _dbresults  1415 switch (dbproc->dbresults_state) {
> e dblib.c _dbresults  1417 dbproc->dbresults_state =
_DB_RES_NO_MORE_RESULTS;
> f dblib.c _dbresults  1421 dbproc->dbresults_state =
_DB_RES_NO_MORE_RESULTS;
> g dblib.c _dbresults  1429 dbproc->dbresults_state = _DB_RES_INIT;
> h dblib.c dbnextrow   1587 dbproc->dbresults_state,
prdbresults_state(dbproc->dbresults_state));
> i dblib.c dbnextrow   1589 if (!resinfo || dbproc->dbresults_state !=
_DB_RES_RESULTSET_ROWS) {
> j dblib.c dbnextrow   1632 dbproc->dbresults_state =
_DB_RES_NEXT_RESULT;
> k dblib.c dbsqlok     4267 dbproc->dbresults_state =
_DB_RES_RESULTSET_EMPTY;
> l dblib.c dbsqlok     4279 dbproc->dbresults_state =
_DB_RES_NEXT_RESULT;
> m dblib.c dbsqlok     4281 dbproc->dbresults_state =
_DB_RES_NO_MORE_RESULTS;
> n dblib.c dbsqlok     4288 dbproc->dbresults_state = _DB_RES_SUCCEED;
> o dblib.c dbwritetext 5888 dbproc->dbresults_state = _DB_RES_INIT;
> p dblib.c dbsqlsend   6358 dbproc->dbresults_state = _DB_RES_INIT;
> q dblib.c dbsqlsend   6378 dbproc->dbresults_state = _DB_RES_INIT;
> r rpc.c   dbrpcsend    272 dbproc->dbresults_state = _DB_RES_INIT;
> 

It seems a trace or a grep... how did you generate it?

> All this "if state A then action B" is unnecessary, as far as I can
> tell.  Whether to free memory or not can always be determined by
> inspecting the pointer for NULL.  Which token to stop on 
> depends on what
> the function's purpose.  But I don't think we understood that when
> dbresults_state got to be so important.  
> 

I can't help here.

> > Oh... have you see (which is the correct english form here...)
> 
> [have you seen]  ;-)
> 

tnx, there's always space for improvements

> > src/odbc/unittests/describecol.c test? It use an external file as
> > some sort of basic script language for test. Mainly is composed by
> > actions (like select, odbc and set commands) and tests (attr). I
> > would like to have a script that could tranform dumps to some sort
> > of test script one day...
> 
> I like this idea.  Do you mean you'd like to extract SQL from 
> a TDSDUMP file?  
>

Well, dump should include informations of called functions so you can
provide a sequence with calls. Well, putting this sequence in a sort of
script and adding some additional tests line (and of course some
comments to explain why the test and so on). Let me make an example
(dump from dblib/rpc test): 

...
09:29:34.926119 (rpc.c:80):dbrpcinit(0x9ef91e0, #t0022, 0)
09:29:34.926239 (rpc.c:126):dbrpcinit() added rpcname "#t0022"
09:29:34.926413 (rpc.c:159):dbrpcparam(0x9ef91e0, @null_input, 0x1, 47,
-1, 0, (nil))
09:29:34.926528 (rpc.c:243):dbrpcparam() added parameter "@null_input"
09:29:34.926663 (rpc.c:159):dbrpcparam(0x9ef91e0, @first_type, 0x1, 47,
64, 0, 0xbfc6c86a)
09:29:34.926774 (rpc.c:243):dbrpcparam() added parameter "@first_type"
09:29:34.926899 (rpc.c:159):dbrpcparam(0x9ef91e0, @nullout, 0x1, 56, -1,
0, 0xbfc6c864)
09:29:34.927007 (rpc.c:243):dbrpcparam() added parameter "@nullout"
09:29:34.927145 (rpc.c:159):dbrpcparam(0x9ef91e0, @nrows, 0x1, 56, -1,
-1, 0xbfc6c860)
09:29:34.927256 (rpc.c:243):dbrpcparam() added parameter "@nrows"
09:29:34.927381 (rpc.c:159):dbrpcparam(0x9ef91e0, @c, 0x0, 39, 0, 0,
(nil))
09:29:34.927490 (rpc.c:243):dbrpcparam() added parameter "@c"
09:29:34.927628 (rpc.c:262):dbrpcsend(0x9ef91e0)
09:29:34.927741 (rpc.c:379):parm_info_alloc(): parameter null-ness = 1
09:29:34.927879 (rpc.c:310):parameter size = 0, data = 0x9ef9a18,
row_size = 0
09:29:34.927983 (rpc.c:328):setting parameter #0 to NULL
09:29:34.928087 (rpc.c:379):parm_info_alloc(): parameter null-ness = 1
09:29:34.928206 (rpc.c:310):parameter size = 0, data = 0x9f2c3a8,
row_size = 0
09:29:34.928309 (rpc.c:328):setting parameter #1 to NULL
09:29:34.928436 (rpc.c:379):parm_info_alloc(): parameter null-ness = 1
09:29:34.928554 (rpc.c:310):parameter size = 0, data = 0x9ef90d0,
row_size = 0
09:29:34.928656 (rpc.c:328):setting parameter #2 to NULL
09:29:34.928761 (rpc.c:379):parm_info_alloc(): parameter null-ness = 0
09:29:34.928917 (rpc.c:310):parameter size = 4, data = 0x9ef9090,
row_size = 0
09:29:34.929024 (rpc.c:315):copying 4 bytes of data to parameter #3
09:29:34.929130 (rpc.c:379):parm_info_alloc(): parameter null-ness = 1
09:29:34.929243 (rpc.c:310):parameter size = 0, data = 0x9f2c408,
row_size = 0
09:29:34.929345 (rpc.c:328):setting parameter #4 to NULL
09:29:34.929454 (util.c:119):Changing query state from IDLE to QUERYING
...
09:29:34.932507 (rpc.c:298):dbrpcsend() returning SUCCEED
09:29:34.932674 (dblib.c:4220):dbsqlok(0x9ef91e0)
...

this can be translated to something like

...
rpcinit '#t0002'
rpcparam '@null_input' OUTPUT SYBINT
rpcparam ...
...
rpcsend
sqlok
...

each command should have arguments with defaults (for instance is
supposed that functions succeed is not specified).
Well, it's only an idea...



About patch
http://freetds.cvs.sourceforge.net/freetds/freetds/src/tds/token.c?r1=1.
313&r2=1.314&sortby=date
these lines

 if( marker == TDS_FAIL ) {
    tdsdump_log(TDS_DBG_FUNC, "error: tds_process_param_result()
returned TDS_FAIL\n");
    return TDS_FAIL;
 }

seem a bit odd.. tds_get_byte does not return TDS_FAIL and also you
should close connection if you detect an problem in server data (but
perhaps tds_get_byte returns invalid only if connection was broken).

freddy77