View mode: basic / threaded / horizontal-split · Log in · Help
January 25, 2006
[OT] efficient return values
Hi,

This really should go in a C++ newsgroup, but I've noticed that I have more 
in common with the people here than on a C++ group. Must be all the 
frustrations with C/C++ and the hope for a better tomorrow. More than hope, 
an actual vision ; )

Here's a construct I can't seem to get right in C++. It works, but I hate 
the way I've implemented it. Maybe somebody has a better idea?

It starts like this: I convert some handle into a class to make use of C++'s 
RAII, ctor/dtor. I do this all the time, but today it happens to be for 
sqlite3_get_table / sqlite3_free_table. Needless to say, the handle (char**) 
obtained by sqlite3_get_table must be freed by sqlite3_free_table. The 
wrapper for this table handle I've called SqliteRS (from Result Set):

class SqliteRS
{
 char** table;
 unsigned int cursor, rows, columns;
public:
 SqliteRS(char** t, int c, int r) : table(t), cursor(c), columns(c), 
rows(r) {}
 ~SqliteRS() { sqlite3_free_table(table); }
....
};

And you obtain an SqliteRS by calling Exec of the class SqliteDB, which 
wraps the database handle, sqlite3* (error handling omitted):

SqliteRS SqliteDB::Exec( const char* zSql )
{
 sqlite3_get_table(pdb, zSql, &t, &r, &c, NULL );
 return SqliteRS(t,c,r);
}

All this to be able to do:

SqliteRS results = database.Exec("select * from X");

And everything should be freed nicely.
The problem of course is that the temporary return value will actually be 
copied into the variable "results", causing the table to be freed when the 
temporary return value gets destructed, leaving "results" with an invalid 
handle. This is the problem I'm trying to solve.

It's noteworthy that Visual Studio optimizes this code, even in debug mode! 
There won't be a temporary return value that gets copied to the actual 
variable. However, it won't compile if I create a private copy-ctor, 
eventhough it's never called when it's public.

At the moment I have two ways around this:

1) create a copy ctor that steals the pointer (sets it to NULL in the 
temporary return value); or
2) create a seperate class for the return value and a ctor that takes this 
class in SqliteRS.

1) would be implemented like this:

SqliteRS::SqliteRS( const SqliteRS &st )
{
 memcpy(this,&st,sizeof st);
 const_cast<SqliteRS&>(st).table = NULL;
}

2) is a bit messier, but basically goes like this: SqliteDB::Exec returns an 
_SqliteRS with only the basic information (char**,int,int in this case). 
_SqliteRS has not dtor. SqliteRS becomes a ctor that takes an _SqliteRS, and 
the dtor that frees the handle. For added DBC one could add an assertion in 
the _SqliteRS dtor to assert that it was in fact copied into an SqliteRS, 
but this would require a const_cast<> similar to 1).

Returning handles from other functions / factories happens only too often, 
and wrapping these handles in a class to make use of RAII (ctor/dtor) seems 
the way to go. But why is this contruction so tricky to do nicely? Am I 
forgetting something?

In D the class will be created on the heap and you're returning the handle 
of the class, so there's never a copy involved. One can even use RAII:

class SqliteDB
{
....
 SqliteRS Exec( char[] sql ) { .... return new SqliteRS(...); }
}

auto SqliteRS result = database.Exec("...");

This works just fine.

Lionello.

By the way, has somebody else noticed that I can put an auto variable (the 
RAII kind) in a global handle and it still gets destructed when the 'auto' 
leaves the scope? Shouldn't the compiler complain?
January 25, 2006
Re: [OT] efficient return values
Lionello Lunesu wrote:
> Hi,
> 
> This really should go in a C++ newsgroup, but I've noticed that I have more 
> in common with the people here than on a C++ group. Must be all the 
> frustrations with C/C++ and the hope for a better tomorrow. More than hope, 
> an actual vision ; )
> 
> Here's a construct I can't seem to get right in C++. It works, but I hate 
> the way I've implemented it. Maybe somebody has a better idea?
> 
> It starts like this: I convert some handle into a class to make use of C++'s 
> RAII, ctor/dtor. I do this all the time, but today it happens to be for 
> sqlite3_get_table / sqlite3_free_table. Needless to say, the handle (char**) 
> obtained by sqlite3_get_table must be freed by sqlite3_free_table. The 
> wrapper for this table handle I've called SqliteRS (from Result Set):
> 
> class SqliteRS
> {
>   char** table;
>   unsigned int cursor, rows, columns;
> public:
>   SqliteRS(char** t, int c, int r) : table(t), cursor(c), columns(c), 
> rows(r) {}
>   ~SqliteRS() { sqlite3_free_table(table); }
> ....
> };
> 
> And you obtain an SqliteRS by calling Exec of the class SqliteDB, which 
> wraps the database handle, sqlite3* (error handling omitted):
> 
> SqliteRS SqliteDB::Exec( const char* zSql )
> {
>   sqlite3_get_table(pdb, zSql, &t, &r, &c, NULL );
>   return SqliteRS(t,c,r);
> }
> 
> All this to be able to do:
> 
> SqliteRS results = database.Exec("select * from X");
> 
> And everything should be freed nicely.
> The problem of course is that the temporary return value will actually be 
> copied into the variable "results", causing the table to be freed when the 
> temporary return value gets destructed, leaving "results" with an invalid 
> handle. This is the problem I'm trying to solve.

> At the moment I have two ways around this:
> 
> 1) create a copy ctor that steals the pointer (sets it to NULL in the 
> temporary return value); or
> 2) create a seperate class for the return value and a ctor that takes this 
> class in SqliteRS.

[snip]

> Returning handles from other functions / factories happens only too often, 
> and wrapping these handles in a class to make use of RAII (ctor/dtor) seems 
> the way to go. But why is this contruction so tricky to do nicely? Am I 
> forgetting something?

This is what std::auto_ptr<> is perfect for. An auto_ptr will, using 
your words, steal the reference being assigned to it. A auto_ptr is 
perfect when you want to transfer responsibility.

> In D the class will be created on the heap and you're returning the handle 
> of the class, so there's never a copy involved. One can even use RAII:
> 
> class SqliteDB
> {
> ....
>   SqliteRS Exec( char[] sql ) { .... return new SqliteRS(...); }
> }
> 
> auto SqliteRS result = database.Exec("...");

Yes, but with this, the user needs to remember to make the reference 
auto. There is (AFAIK) no way in D to make this automatic like the c++ 
auto_ptr does. If D had more power (assignment overloading, struct 
destructors/constructors), an auto_ptr would be straight-forward to 
implement in D too.

/Oskar
January 25, 2006
Re: [OT] efficient return values
>> auto SqliteRS result = database.Exec("...");
>
> Yes, but with this, the user needs to remember to make the reference auto. 
> There is (AFAIK) no way in D to make this automatic like the c++ auto_ptr 
> does. If D had more power (assignment overloading, struct 
> destructors/constructors), an auto_ptr would be straight-forward to 
> implement in D too.

Maybe the prototype of the Exec function could already mention the 'auto'?

auto SqliteRS Exec( char[] sql ) { ... }

At the moment the compiler (correctly) complains with "functions cannot be 
const or auto".

This kind of "auto" could also be used to force the caller to catch the 
return value in a variable. I guess exceptions can be used to prevent the 
caller from ignoring the result value.

Exec("select * from X"); // not allowed; use ExecNonQuery

Lio.
January 25, 2006
Re: [OT] efficient return values
Lionello Lunesu wrote:
> 
> Returning handles from other functions / factories happens only too often, 
> and wrapping these handles in a class to make use of RAII (ctor/dtor) seems 
> the way to go. But why is this contruction so tricky to do nicely? Am I 
> forgetting something?

A lot of it has to do with the messed-up way the ODBC API was 
written--it just doesn't map well into an object model.  What I've done 
in the past is store the handles in an RAII-type object as you are:

class db_handle_ptr :
private	noncopyable
{
public:
	SQLHDBC	hdbc;
	db_handle_ptr() : hdbc( SQL_NULL_HDBC ) {}
	~db_handle_ptr() {
            ::SQLDisconnect( hdbc );
            ::SQLFreeHandle( SQL_HANDLE_DBC, hdbc );
        }
};

And then pass around references to it using a shared pointer (similar to 
the one available in Boost).  I get around returning handles at all by 
instead returning a temporary object that is only accessible by the 
destination object:

class resultset :
private	noncopyable
{
public:
    class resultset_init
    {
    public:
        resultset_init( shared_ptr<db_handle_ptr> const& hdb,
                        shared_ptr<stmt_handle_ptr> const& hs );

    private:
                friend resultset;

                shared_ptr<db_handle_ptr>    m_db;
                shared_ptr<stmt_handle_ptr>  m_stmt;
    };

    resultset( resultset_init const& init );

private:
    shared_ptr<db_handle_ptr>   m_db;
    shared_ptr<stmt_handle_ptr> m_stmt;
};

class statement :
private noncopyable
{
public:
    resultset::resultset_init open();
};

The end result is that all the handles that need to stay alive do so 
until all dependent objects are destroyed.  So even if the connection 
object is deleted, any open statements or resultsets will remain usable.


Sean
January 25, 2006
Re: [OT] efficient return values
Oskar Linde wrote:
> 
> This is what std::auto_ptr<> is perfect for. An auto_ptr will, using 
> your words, steal the reference being assigned to it. A auto_ptr is 
> perfect when you want to transfer responsibility.

Yes, but it's not perfect if you wish to simply store a reference to 
something as a class member, because auto_ptr performs a destructive 
copy.  Just something to be aware of for those who haven't used auto_ptr 
before.


Sean
January 27, 2006
Re: [OT] efficient return values
When I think about it....

Shouldn't an ignored return value be treated as an error?

Or at least a warning, or an assertion (like forgetting to return 
something). Maybe adding a construction like:

void = I_dont_care_about_the_return_value();

L.
January 27, 2006
Re: [OT] efficient return values
Lionello Lunesu wrote:
> When I think about it....
> 
> Shouldn't an ignored return value be treated as an error?
> 
> Or at least a warning, or an assertion (like forgetting to return 
> something). Maybe adding a construction like:
> 
> void = I_dont_care_about_the_return_value();
> 
> L. 

The problem is, not all return values matter.
There are lots of APIs which have some really stupid return values. The 
classic example being printf().

void = printf("hello world\n");

No thanks.
I think you'd need some kind of "must use" syntax on a function 
declaration, which would trigger an error if it wasn't used. Maybe a 
"nonvoid" keyword.

But I don't think this is a very common programming error. Not really 
worth worrying about.
January 27, 2006
Re: [OT] efficient return values
> void = printf("hello world\n");

You shouldn't be using printf to begin with : S

But I agree, not a common mistake. I was thinking about the waste of CPU 
time when somebody discards return codes. Fits nicely with D's the notion of 
"statement must have side effect", for example a "{strlen(a);}" compiles 
fine, but is a waste of time.

I made the following template when I noticed that many colleagues were 
ignoring the return value of some Lock() call:

//! Assert that a (return) value gets used (used for handles)
template <typename T>
class _use
{
 const T retval;
 bool unused_return_value;
public:
 _use( const T& rv ) : retval(rv), unused_return_value(false) {}
 ~_use() { assert(unused_return_value); }
 operator T () { unused_return_value=true; return retval; }
};

I defined the call as _use<bool> Lock() and it now asserts when the return 
value is not caught into some variable or expression. I know, I should have 
used C++ exceptions, which I hope to do when we get more time on our 
hands... (probably never :S)

Lio.
January 27, 2006
Re: [OT] efficient return values
Lionello Lunesu wrote:
> 
> I made the following template when I noticed that many colleagues were 
> ignoring the return value of some Lock() call:
> 
> //! Assert that a (return) value gets used (used for handles)
> template <typename T>
> class _use
> {
>   const T retval;
>   bool unused_return_value;
> public:
>   _use( const T& rv ) : retval(rv), unused_return_value(false) {}
>   ~_use() { assert(unused_return_value); }
>   operator T () { unused_return_value=true; return retval; }
> };
> 
> I defined the call as _use<bool> Lock() and it now asserts when the return 
> value is not caught into some variable or expression. 


I know, I should have
> used C++ exceptions, which I hope to do when we get more time on our 
> hands... (probably never :S)

I'm not sure about that. This is a logical error, and ideally it would 
be caught at compile time. I think an assert is the next best option.
(Because even if the exception is caught, there's still a logic error).

I'm a little biased against exceptions, because the worst bug I've ever 
encountered in my life was caused by a destructor which threw an 
exception, inside a multi-threaded program. (There were virtually no 
symptoms, that single thread just silently disappeared - yikes).
January 27, 2006
Re: [OT] efficient return values
Don Clugston wrote:
> 
> I'm a little biased against exceptions, because the worst bug I've ever 
> encountered in my life was caused by a destructor which threw an 
> exception, inside a multi-threaded program. (There were virtually no 
> symptoms, that single thread just silently disappeared - yikes).

That in itself is basically an illegal operation in C++, since throwing 
an exception while another is in flight terminates the program.  If in 
doubt, wrap the contents of your dtor in a try{}catch(...){} block, and 
report the error by some other means.  I'll admit I do like that D 
doesn't have this problem, even if it means that some exceptions are 
overridden by others and are therefore never reported.  I've wondered 
whether exception chaining via the .next property might be right for 
this purpose, but it's not a perfect fit.


Sean
« First   ‹ Prev
1 2
Top | Discussion index | About this forum | D home