[vlc-devel] [PATCH] SQLite module

[Rémi Denis-Courmont]

On Wed, 25 Nov 2009 16:26:17 +0530, Srikanth Raju <srikiraju at gmail.com>
wrote:
> We can include sqlite in the contribs.
> MacPorts had an old version of sqlite and a new version was required for
> testing. So, with-sqlite was included. Shall I remove it and include
> sqlite in the contribs? Or do we want to keep the contribs minimal?

We are going to need it in the contrib anyway, if only for Windows, no?

>> > +static char* VMSprintf( const char* psz_fmt, va_list args )
>> > > +{
>> > > +    char *psz = sqlite3_vmprintf( psz_fmt, args );
>> > > +    char *ret = strdup( psz );
>> > > +    sqlite3_free( psz );
>> > > +    return ret;
>> > > +}
>> > >
>> > > Do we really need this double copy/allocation?
>> >
>> > Yes. psz is allocated with sqlite3_malloc. Some "fastmem" builds can
>> > use different allocation strategies, hence we cannot rely on free()
>> > to free psz. Hence the copy.
>>
>> That's not the point. Why do you need strdup and free?
>>
> Is there a way to do this without strdup?

What's wrong with using sqlite3_free() directly _after_ you are done?

>> > > Same stalling problem.
>> > >
>> > > IMHO, RollbackTransaction should return void, because it does not
>> > > make much sense to fail here.
>> >
>> > I agree that it should return void. However, the people who made
>> > sqlite decided that a ROLLBACK can fail if there are pending
>> > reads/writes.

>> I don't care what the SQLite people decided. There _must_ be a way to
>> recover from the error. Keeping a stalled mutex is _not_ acceptable
>> under any circumstances. In fact, it will cause an assertion failure
>> when you close the DB.
>>
>> Ah. The Rollback needs to be called again after a while.

Oh noes. API users will get it wrong. Please return void and fix the error
internally yourself.

-- 
Rémi Denis-Courmont