Hi,<br><br>I noticed that CommitTransaction needs a return code as commits can fail<br>and so does RollbackTransaction.<br>I've attached the patch for vlc_sql.h<br><br><div class="gmail_quote">
On Wed, Sep 23, 2009 at 11:54 PM, Laurent Aimar <span dir="ltr"><<a href="mailto:fenrir@via.ecp.fr" target="_blank">fenrir@via.ecp.fr</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
> + sqlite3 *db; /**< Database connection. */<br>
> + vlc_mutex_t lock; /**< SQLite mutex. Threads are evil here. */<br>
I see it is used to protect some of the sql_t functions.<br>
If it is wanted to be able to call sql_t from multiple threads, I would like<br>
first a patch to vlc_sql.h specifying those requirements.<br>
<br></blockquote><div><br>Not exactly sure how you want me to specify this. I have included that<br>the functions are threadsafe in the documentation in the first patch<br>(attached).<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><br>
> +static int GetTables( sql_t * p_sql,<br>
> + char *** result )<br>
> +{<br>
> + int nrow, i_num = false;<br>
> + char ***pppsz_res;<br>
> +<br>
</div><div>> + vlc_mutex_lock( &p_sql->p_sys->lock );<br>
> +<br>
> + if( !p_sql->p_sys->db )<br>
</div> Same.<br>
<div>> +<br>
> + if( result )<br>
> + pppsz_res = result;<br>
> + else<br>
> + pppsz_res = (char ***) malloc( sizeof( char ** ) );<br>
</div> What is the purpose of pppsz_res ? Is there an interest to call<br>
GetTables with result == NULL ? If so, documentation in vlc_sql.h is<br>
welcome.<br>
<br></blockquote><div><br>Correct. Makes no sense at all. Removed.<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
I see that 2 locks are used:<br>
- lock for non transaction functions<br>
- trans_lock for transaction functions<br>
I don't think that works as you want.<br>
<br></blockquote><div><br>Ok I've fixed this, I think.<br><br>I've locked the transaction queries using lock as well.<br>The trans_lock is to ensure that transactions are already not running from the<br>given thread. This isn't actually "required", as the BeginTransaction will fail<br>
if a transaction is already running. But using mutexes for this makes the<br>task of running a transaction much easier, as the user will not have to<br>keep testing the return value of BeginTransaction until the transaction starts.<br>
<br>lock was used for sqlite threadsafety, which should be there for all queries,<br>including the transaction control queries.<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Unless sqlite is already reentrant and no lock are needed, or it is not and<br>
then non transaction and transaction functions probably need to be protected<br>
by the same lock.<br>
<div><div></div><div><br></div></div></blockquote><div><br>Not all sqlite are threadsafe. If the sqlite is compiled with the threadsafe option,<br>then the functions are threadsafe. It is possible to compile sqlite<br>
without thread safety. So, instead of testing for whether the given sqlite is threadsafe<br>all the time and then locking using mutex conditionally, I have used locks for all<br>queries. This shouldn't be too much of an overhead.<br>
<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div><div>
Regards,<br>
<br>
--<br>
fenrir<br>
<br>
_______________________________________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="http://mailman.videolan.org/listinfo/vlc-devel" target="_blank">http://mailman.videolan.org/listinfo/vlc-devel</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>Regards,<br>Srikanth Raju<br>