FreeTDS Development Group

["James K. Lowden" <jklowden AT freetds.org>]

Frediano Ziglio wrote:
> 
> Test to send query is 
> 
> if (tds_set_state(tds, TDS_QUERYING) != TDS_QUERYING)
>       return TDS_FAIL;
> 
> this is bad. If state was already TDS_QUERYING it try to set state, in
> tds_set_state

if (tds->state != TDS_QUERYING) {
        if (tds_set_state(tds, TDS_QUERYING) != TDS_QUERYING)
                return TDS_FAIL;
}

> case TDS_QUERYING:
> 
>     if (tds->state == TDS_DEAD) {
>       tds_client_msg(tds->tds_ctx, tds, 20006, 9, 0, 0, "Write to SQL
> Server failed.");
>       return tds->state;
>     } else if (tds->state != TDS_IDLE) {
>       tdsdump_log(TDS_DBG_ERROR, "tds_submit_query(): state is PENDING
> \n");
>      tds_client_msg(tds->tds_ctx, tds, 20019, 7, 0, 1,
>                     "Attempt to initiate a new SQL Server operation with
> results pending.");

With my error handler work, the client will be able to cancel the
operation, and tds->state will become TDS_IDLE.  

>      return tds->state;
>     }
> 
> so it give error but continue... quite bad (mainly for thread reasons).
> Perhaps tds_set_state should return TDS_SUCCEED/TDS_FAIL ??

I think it's better as it is, because we can write code like:

        switch (tds_set_state(tds, TDS_QUERYING)) {
        case TDS_QUERYING: /*...*/
        case TDS_IDLE: /*...*/
        default: /*...*/
        }

I'm not a big fan of boolean return values.  Not getting what you expected
isn't always the same as failing.   

Regards, 

--jkl