FreeTDS Development Group

["Castellano, Nicholas" <Nicholas_Castellano AT acml.com>]

My original thoughts were along these lines...but as I looked further, I
realized that tds_connect() really needs to be able to change the state at
different points depending on various conditions.  The TDSSOCKET shouldn't
appear live until after the connect() call, and if the login fails it should
appear dead when the "Login incorrect." message is sent.  So breaking up the
calls in the ways that you suggest really don't provide the degree of
control that I think is needed.

Here's an excerpt from my version, showing the critical sections where the
state needs to change.  Note that the two calls to tds_client_msg() shown
below need the state to be dead, while any calls to tds_client_msg()
performed from tds_process_login_tokens() need the state to be live:

        if (connect(tds->s, (struct sockaddr *) &sin, sizeof(sin)) <0) {
                char message[128];
                sprintf(message, "src/tds/login.c: tds_connect: %s:%d",
                        inet_ntoa(sin.sin_addr), ntohs(sin.sin_port));
                perror(message);
                tds_client_msg(tds->tds_ctx, tds, 20009, 9, 0, 0,
                        "Server is unavailable or does not exist.");
                tds_free_config(config);
                tds_free_socket(tds);
                *tdsp = NULL;
                return;
        }
        }
        /* END OF NEW CODE */

        /* We are now connected!
         * Save our TDSSOCKET now, to ensure that we don't appear
         * to be DBDEAD() upon entry to error/message handlers.
         */
--->    *tdsp = tds;

        if (IS_TDS7_PLUS(tds)) {
                tds->out_flag=0x10;
                tds7_send_login(tds,config);
        } else {
                tds->out_flag=0x02;
                tds_send_login(tds,config);
        }
        if (!tds_process_login_tokens(tds)) {
                /* Login incorrect...mark the socket dead. */
--->            *tdsp = NULL;
                tds_client_msg(tds->tds_ctx, tds, 20014, 9, 0, 0,
                        "Login incorrect.");
                tds_free_config(config);
                tds_free_socket(tds);
                return;
        }

I can't think of any way to do this properly without using a pointer to a
TDSSOCKET ** to access the caller's state at the critical points.

So, I'd still like to go ahead with the change as I originally proposed.

Cheers,
--nick

-----Original Message-----
From: bounce-freetds-145195 AT franklin.oit.unc.edu
[mailto:bounce-freetds-145195 AT franklin.oit.unc.edu] 
Sent: Wednesday, September 11, 2002 10:18 PM
To: TDS Development Group
Subject: [freetds] Re: WARNING: Changing tds_connect() API



On second look maybe we should simply do it like:

dbproc->tds_socket = tds_alloc_socket(g_dblib_ctx->tds_ctx, 512);
tds_set_parent(dbproc->tds_socket, (void *) dbproc);

if (tds_connect(login->tds_login, dbproc->tds_socket)==TDS_FAIL)
        tds_free_socket(dbproc->tds_socket);

The only changes required to tds_connect is to 

1) remove the context and parent arguments
2) add TDSSOCKET * argument
3) remove the calls to tds_alloc_socket and tds_set_parent from inside
4) reference tds->context instead of using the context argument
5) change the return of tds_connect to simple TDS_SUCCEED/TDS_FAIL

It adds very little work for the API, and simplifies tds_connect a bit.  
Also, if we end up doing it this way, we should change the signature to 
int tds_connect(TDSSOCKET *tds, TDSLOGIN *login) for consistancy (that is 
put TDSSOCKET * as the first parameter like all the other functions).

Thoughts?

Brian

On Wed, 11 Sep 2002, Castellano, Nicholas wrote:

> I will be changing the API for tds_connect().  Currently, this 
> function returns the TDSSOCKET * to its caller.  However, in dblib we 
> need this information in the dbproc before tds_connect() returns.  
> When error or message handlers are invoked during login, the dbproc 
> currently appears to be DBDEAD() since the socket is not set up. In 
> the new scheme, tds_connect will take a TDSSOCKET ** parameter, and 
> will fill in the new TDSSOCKET ** when it has been connected to the 
> database server.
> 
> So instead of:
> 
> dbproc->tds_socket = (void *) tds_connect(login->tds_login,
> g_dblib_ctx->tds_ctx, (void *)dbproc);
> 
> We will now have:
> 
> tds_connect(&(dbproc->tds_socket), login->tds_login, 
> g_dblib_ctx->tds_ctx, (void *)dbproc);
> 
> Of course all the other calls throughout the tree will be updated as 
> well, to use the new API.
> 
> I intend to commit this early tomorrow (along with a while bunch of 
> other error/message handler fixes), unless there are any serious 
> objections.
> 
> Cheers,
> --nick
> 


---
You are currently subscribed to freetds as: [Nicholas_Castellano AT acml.com]
To unsubscribe, forward this message to
$subst('Email.Unsub')


The information contained in this transmission may contain privileged and
confidential information and is intended only for the use of the person(s)
named above.  If you are not the intended recipient, or an employee or agent
responsible for delivering this message to the intended recipient, any
review, dissemination, distribution or duplication of this communication is
strictly prohibited. If you are not the intended recipient, please contact
the sender immediately by reply e-mail and destroy all copies of the
original message. Please note that for certain accounts we do not accept
orders and/or instructions by e-mail, and for those accounts we will not be
responsible for carrying out such orders and/or instructions. Kindly refrain
from sending orders or instructions by e-mail unless you have confirmed that
we accept such communications for your account. Please also note that to
satisfy regulatory requirements we review the outgoing and incoming e-mail
correspondence of staff members serving certain functions.