[vlc-devel] [PATCH] Prepared statements for SQL

[Laurent Aimar]

On Tue, Nov 03, 2009, jpd at videolan.org wrote:
> On Tue, Nov 03, 2009 at 08:02:39PM +0100, Laurent Aimar wrote:
> > On Tue, Oct 27, 2009, Srikanth Raju wrote:
> > >      > +#define VLC_SQL_ROW 1
> > >      > +#define VLC_SQL_DONE 2
> > >      ?What are they used for? A doxy comment would be usefull.
> > >    They're used as return values for sql_Run(). Documented.
> 
> Only two values? Wouldn't it make more sense to make it a bool then?
 sql_Run can also return an error code (VLC_E*).

> >  The problem with sql_type_e is that the size of the storage of an
> > enum is compiler dependant and so the ABI would easily be broken. Now,
> > I don't feel strongly about it, anyone cares to comment ?
> 
> I'll take an unsigned or unsigned short to taste, thanks.
 Unless there is a strong memory gain (not the case here), I don't think using
short is useful. And for an enum, I think it is better to use int (I am unsure
but I think that an enum value is interpreted as signed).

> > > +    /** Create a statement object */
> > > +    sql_stmt_t* (*pf_prepare) ( sql_t* p_sql, const char* psz_fmt,
> > > +                                int i_length );
> >  Please, change the psz_ prefix into p_ to avoid confusion as it is not always
> > a 0 terminated string.
> 
> That, and another unsigned for the length please. size_t in this case.
> If the parameter cannot be meaningfully negative it shouldn't be signed.
 If < 0, strlen() is used instead (usefull).

Regards,

-- 
fenrir