Re: [PATCH] [PUSHED:3-5] fdo#46163 database form listbox only for VARCHAR

On Tue, Feb 21, 2012 at 01:03:09PM +0100, Petr Mladek wrote:
> Lionel Elie Mamane píše v Čt 16. 02. 2012 v 13:57 +0100:

> > Just reported and fixed fdo#46163. A list box is a control that is
> > supposed to match two sets of data:

> >  An entry in the "list content" property is matched to the
> >  corresponding entry in the "list entries" property (yeah, that's
> >  rather confusing naming...). The "list entries" entry is shown to the
> >  user, but the "list content" entry is stored in the database.

> > Attached patch 0003-fdo-46163-convert-bound-values-to-bound-column-s-typ.patch
> > makes sure the values are converted to the right type before being
> > compared to the value in the database.

> Looks sane => pushed
> http://cgit.freedesktop.org/libreoffice/core/commit/?h=libreoffice-3-5&id=b5f33bb8fa14afa17c4694d587215cab4756aa1f

> > Attached patch
> > 0001-ORowSetValue-setTypeKind-correctly-convert-to-C-LOB-.patch
> > avoids a crash under some conditions that can be triggered after
> > application of other patch: instead of assuming the ORowSetValue
> > already contains an Any, construct one if necessary. Note that the
> > other cases already do type conversion "nicely", e.g. string to int or
> > 8-bit int to 32-bit int.

> To be honest, I am not 100% sure that I understand everything.

ORowSetValue is used to store in-memory a "database value", as a
tagged union (a struct of an union (m_aValue) and a int field
(m_eTypeKind) whose value says which member of the union to use), plus
some other information. Some types are stored directly in the union,
but others are allocated dynamically and a pointer to it is stored in
m_aValue.m_pValue. Of these types, some are stored as (a pointer to) a
::com::sun::star::uno::Any.

It is also used for the "list content" entries of a listbox, with type
tag "VARCHAR" (strings).

Now, 0003-fdo-46163-convert-bound-values-to-bound-column-s-typ.patch
introduces a conversion from these strings to the datatype of the
underlying column, e.g. INTEGER (32bit integer), by calling
"setTypeKind(INTEGER)". In this case, setTypeKind in turn calls
geInt32(), which calls ::rtl::OUString::toInt32, i.e. parses the
string as a number in base 10.


Now, if the column is of type "CLOB" instead? setTypeKind(CLOB) is
called, which used to call getAny():

  ::com::sun::star::uno::Any   getAny() const { 
return*(::com::sun::star::uno::Any*)m_aValue.m_pValue; }

But in this case, m_aValue.m_pValue contains an OUString, not an Any,
so this leads to a crash or garbage data.


This patch changes it to calling makeAny() instead of getAny(), which
will cleanly do:

                Any rValue
                OSL_ENSURE(m_aValue.m_pString,"Value is null!");
                rValue <<= (::rtl::OUString)m_aValue.m_pString;
                return rValue;

That is, sticking the OUString in an Any, which setTypeKind will then
assign (via operator=(const Any&)) to a newly allocated Any and stick
the pointer to that in m_pValue.

> Anyway, the patch does sensible things. I did not find any logical
> or technical problem. It made my test document working better => I
> have pushed it to
> the 3-5 branch:
> http://cgit.freedesktop.org/libreoffice/core/commit/?h=libreoffice-3-5&id=b5f33bb8fa14afa17c4694d587215cab4756aa1f

Great, thanks.

> It might be better if you attach a simple test document instead of
> describing complex steps how to create it.

Yes, good point. I will in future.

-- 
Lionel