List:MySQL++« Previous MessageNext Message »
From:Quentin Armitage Date:June 1 2014 8:18am
Subject:Re: Connections getting "lost"/overwritten
View as plain text  
On Sat, 2014-05-31 at 17:21 -0600, Warren Young wrote:

> On May 31, 2014, at 7:29 AM, Quentin Armitage <quentin@stripped> wrote:
> 
> > The following code demonstrates that an established connection can be
> > partially overwritten. 
> 
> Thanks for the patch!  One thing, though:
> 
> > -is_connected_(false)
> 
> ...
> 
> > -	else {
> > -		is_connected_ = false;
> > -	}
> > }
> 
> Doesn’t this pair ensure that if neither Connection object is connected, that
> is_connected_ gets a random value?  Perhaps I have missed something.
> 

I think it doesn't get a random value, due to the addition of "if
(connected()) disconnect();" which means that if is_connected_ is set,
disconnect() will be called and set is_connected_ false. However, the
reasoning I used for deleting the is_connected_(false) initialisation
was wrong (I had failed to think of it in terms of this being a
constructor, and erroneously thought it was potentially clearing
is_connected when there was an existing connection), and removing the
initialisation of is_connected(false) does mean that disconnect() may
wrongly be called.

> Also, please follow the style you find in existing code when making a patch.  (Not
> just to MySQL++.  The rule is good for all projects.)  In MySQL++, we use braces always,
> even when C++ doesn’t require them.

Apologies for this. I did attempt (as I always do) to follow the coding
style; I just hadn't occurred to me about the braces.

Revised patch:
=========
Index: dbdriver.cpp
===================================================================
--- dbdriver.cpp	(revision 2767)
+++ dbdriver.cpp	(working copy)
@@ -127,12 +127,13 @@
 void
 DBDriver::copy(const DBDriver& other)
 {
+	if (connected()) {
+		disconnect();
+	}
+
 	if (other.connected()) {
 		connect(other.mysql_);
 	}
-	else {
-		is_connected_ = false;
-	}
 }
 
 
By the way, while testing this, I noticed that in connection.cpp some
functions, such as ping and select_db, check that the Connection object
is connected prior to sending a query, whereas others, such as create_db
and count_rows do not check. Also exec/execute/store/use do not check if
the connection is connected. Would you be interested in a patch that
adds checks for connected?

Quentin Armitage

Thread
Connections getting "lost"/overwrittenQuentin Armitage31 May 2014
  • Re: Connections getting "lost"/overwrittenWarren Young31 May 2014
    • Re: Connections getting "lost"/overwrittenQuentin Armitage1 Jun 2014
      • Re: Connections getting "lost"/overwrittenWarren Young1 Jun 2014
        • Re: Connections getting "lost"/overwritten & checking connected()Quentin Armitage3 Jun 2014
          • Re: Connections getting "lost"/overwritten & checking connected()Warren Young3 Jun 2014