Date: prev next · Thread: first prev next last
2012 Archives by date, by thread · List index


Hi,

The following code does not do what one would think:

 sal_uInt8 nValue = 255;
 Any aValue;
 aValue <<= nValue;

aValue now contains a *BOOLEAN*. That's rather... surprising, and
could bite any of us one day. In particular:

 sal_uInt8 naValue;
 aValue >>= naValue;

now naValue contains 1. Not 255..

Ideally, I'd like that we fix this in LibreOffice 4.1.

The cleanest would be to add support for unsigned 8 bit integer in
Any, which as far as I understand would entail:

 - adding typelib_TypeClass_UNSIGNED_BYTE to cppu/inc/typelib/typeclass.h

 - adding "UNSIGNED_BYTE" to udkapi/com/sun/star/uno/TypeClass.idl (published!)

I'm not sure what the consequences of the latter are. E.g. do all Uno
bindings/bridges/... (Java, StarBasic, Python, ...) have to be updated
one by one?

Also, it seems the basic mechanism of dispatch with Any is C++
overloads; in particular, this is what happens in:

 aValue <<= nValue

But sal_uInt8 and sal_Bool are typedefs of the same type, namely
"unsigned char", so I don't know how to make the distinction between
sal_Bool and sal_uInt8 in there.

So frankly, I'm not sure what to do... If we were a
pure C++ program, I'd change:

typedef unsigned char sal_Bool;

into

typedef bool sal_Bool;

or maybe

class sal_Bool
{
    unsigned char m_bValue;
    // constructors, member functions, operators, etc so that
    // everything works right
}


But we have a "pure C" layer, so no can no do. In my understanding,
even C99's _Bool type won't save us, for several reasons:

1) I'm not sure it is really a "different type" than the other integer
   types
2) It is not available in C++, so we can't have it in C++ code?
3) MSVC doesn't have C99 anyway


Hmm... Could we do something like:

typedef struct _sal_Bool
{
    unsigned char bValue;
#ifdef __cplusplus
    inline operator bool() const
    {
       return bValue;
    }
    inline _sal_Bool(bool b)
      : bValue(b)
    { }
    inline void operator=(bool b)
    {
      bValue=b;
    }
#endif
} sal_Bool;
#   define sal_False ((sal_Bool){0})
#   define sal_True  ((sal_Bool){1})
#ifdef __cplusplus
bool operator==(sal_Bool b0, sal_Bool b1)
{
  return b0.bValue == b1.bValue;
}
#endif

??

So, in pure C code, we can still write:

 sal_Bool b0 = sal_False;
 sal_Bool b1 = sal_True;
 b0 = b1;

but we'd have to write:

 if (b0.bValue) {}
 if (b0.bValue == b1.bValue) {}
 b0.bValue = A || B && C

On the other hand, in C++ code, we could write:

 if (b0 == sal_False) {}
 if (b0 == b1) {}
 if (b0) {}
 b0 = A || B && C

The other solution would be an enum? Makes things a bit easier in C,
but less in C++. All these would work in C:

 if (b0 == sal_False) {}
 if (b0 == b1) {}
 if (b0) {}

but not

 b0 = A || B && C

and I don't think we can get it to work in C++, either.

So I'd rather make things maximally easy in C++, which is our main
language :)

Since we are in our "unstable API/ABI" period, *if* we can have a
clean solution, let's have it for LibreOffice 4.1. Since the in-memory
layout of sal_Bool does not change (right?), this might even be fully
ABI-compatible? (but not API-compatible for C, but API-compatible for
C++)


In terms of more hackish solutions, I have thought of not normalising
sal_Bool in Any (to make them "true" synonyms of sal_uInt8), *but*
this would break any code of the form:

 if (b0 == sal_True) {}
 switch (b0)
 {
    case sal_False:
    case sal_True:
 }

so, bleh, not looking forward to it. It would be something like:

diff --git a/cppu/inc/com/sun/star/uno/Any.hxx
b/cppu/inc/com/sun/star/uno/Any.hxx
index 6f3bda9..62ca967 100644
--- a/cppu/inc/com/sun/star/uno/Any.hxx
+++ b/cppu/inc/com/sun/star/uno/Any.hxx
@@ -248,7 +248,7 @@ inline sal_Bool SAL_CALL operator >>= ( const ::com::sun::star::uno::Any & rAny,
 {
     if (typelib_TypeClass_BOOLEAN == rAny.pType->eTypeClass)
     {
-        value = (* reinterpret_cast< const sal_Bool * >( rAny.pData ) != sal_False);
+        value = * reinterpret_cast< const sal_Bool * >( rAny.pData );
         return sal_True;
     }
     return sal_False;
diff --git a/cppu/source/uno/copy.hxx b/cppu/source/uno/copy.hxx
index ce46492..d532d21 100644
--- a/cppu/source/uno/copy.hxx
+++ b/cppu/source/uno/copy.hxx
@@ -179,7 +179,7 @@ inline void _copyConstructAnyFromData(
         break;
     case typelib_TypeClass_BOOLEAN:
         pDestAny->pData = &pDestAny->pReserved;
-        *(sal_Bool *)pDestAny->pData = (*(sal_Bool *)pSource != sal_False);
+        *(sal_Bool *)pDestAny->pData = *(sal_Bool *)pSource;
         break;
     case typelib_TypeClass_BYTE:
         pDestAny->pData = &pDestAny->pReserved;



Why did I get into this? Well, see https://gerrit.libreoffice.org/#/c/1164/


-- 
Lionel

Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.