[Libreoffice] [PATCH] Removed dependencies on tools/solar.h

[Soeren Moeller]

Ok, so it seems like at the moment substituting ULONG with
sal_uIntPtr, is the safest solution, although it maybe in a next step
would be a good idea to look at, where it can be replaced with a
better fitting datatype. I guess that replacing ULONG with other
things than sal_uIntPtr also would require correcting all callsites of
the changed functions, as this supposedly would break (or at least do
a cast, which possible could go wrong), while the typedef in
types/solar.h at the moment means that calling a function taking
sal_uIntPtr with a ULONG argument still works (and hence ULONG can be
replaced one class at a time).

Do I understand correctly that I without danger can replace BOOL with
bool in non-virtual functions, but should use sal_Bool or change to
bool in all childs of the class at the same time for virtual
functions?

Sören

2011/1/4 Norbert Thiebaud <nthiebaud at gmail.com>:
> On 1/4/11, Soeren Moeller <soerenmoeller2001 at gmail.com> wrote:
>> Thank you for your replies, for the future I will use bool when
>> substituting BOOL outside of the UNO API.
>>
>> With respect to ULONG, I, as Norbert correctly observed, used
>> sal_uIntPtr because of the typedef in tools/solar.h, but what should I
>> use instead? It seems there are the following possibilities:
>> - unsigned long
>> - sal_uInt32 (could be too short on 64-bit systems)
>
> sal_uInt32 is probably what was originally intended with most of these
> ULONG (back then there was no 64 bits system).
> the problem is that changing ULONG to sal_uInt32, like changing BOOL
> with bool can pose problem for virtual function. (as in: one need to
> change the instance of a virtual function in all the subclass at once
> for a given argument to avoid breaking the inheritance model)
>
>
>> - sal_uIntPtr (wrong)
> sal_uIntPtr is not wrong here. it is was is _really_ behind ULONG
> today so that is the safest substitution, I think. Although I agree
> with Kohei that they look weird. When I see uintptr I always think
> that someone is using a dirty trick somewhere storing this into a
> pointer. In other words this is misleading.
>
>
>> - C99 data types (but which of these, and as Norbert writes this would
>> require a compat.h for Microsoft compilers, and I have no idea how to
>> do this (and no Microsoft compiler to test it))
>
> I favor this of course, but that would require some consensus. Either
> we commit to use standard type or we stick with sal_xxxx, but there is
> no benefit in mixing the 2 forever.
> (note: in include magic is really not that hard, it can be stuck in
> solar.h to hide MS breakages)
>
> Note that would still not solve the dilemma: today ULONG is defined a
> uintptr_t so whether we use sal_uIntPtr or uintptr_t does not make any
> difference in that matter.
>
> Norbert
>
>>
>> I would like to do similar removements of dependencies on tools/ in
>> other files in sc/inc/ (and the related source files), so
>> it would be nice to know. which data types I should use instead.
>>
>> Regards
>> Sören
>>
>>
>> 2011/1/4 Norbert Thiebaud <nthiebaud at gmail.com>:
>>> On Mon, Jan 3, 2011 at 10:36 PM, Kohei Yoshida <kyoshida at novell.com>
>>> wrote:
>>>> On Mon, 2011-01-03 at 21:47 +0100, Soeren Moeller wrote:
>>>>> Hi
>>>>>
>>>>> I have removed dependencies on tools/solar.h in some files in sc
>>>>> (according to
>>>>> http://wiki.documentfoundation.org/Easy_Hacks#write_tools.2F_pieces_out
>>>>> ) please review and commit.
>>>>
>>>> Thanks, pushed!
>>>>
>>>> BTW, we generally prefer the standard bool over sal_Bool, so I replaced
>>>> sal_Bool with bool in your patch.  The only place we need to use
>>>> sal_Bool is when dealing with the UNO API.  Other than that, the
>>>> standard boolean type is preferred.
>>>>
>>>> Also, it's a bit weird to use sal_uIntPtr which isn't used much in our
>>>> code base.  So I replaced that with sal_uInt32.
>>>
>>> Kohei,
>>>
>>> I have not read the related code, but in principle uintptr_t and
>>> int32_t are not interchangeable.
>>>  int32_t is 32 bit long, uintptr_t is supposed to be the same size
>>> than void* (that is 32 or 64 bits)
>>>
>>> in our sources,
>>> ULONG is typedef'ed as sal_uIntPrt (in tools/solar.h) , which is wrong (*)
>>> but that explain why Soeren used sal_uIntPtr.
>>>
>>> (*) it is wrong because there are multiple model of 64 bits support.
>>> Notoriously, Microsoft, as usual, instead of fixing their 64 bits
>>> support bugs, have, once again, turn their bugs into a standard and
>>> use the so-called LLP64 model, in which sizeof(long) != sizeof(void*)
>>> Note that ULONG is defined at multiple place, most of them as unsigned
>>> long (which conflict with the main definition of ULONG = sal_uIntPtr).
>>> Which raise the following question: has anyone successfully built
>>> LibreOffice for Win64 ?
>>>
>>>
>>> Note:
>>> C99 has been a standard for quite a while now. why are we not using
>>> the standardized type for these. that is:
>>> int8_t uint8_t, int16_t, uint16_t, int32_t, uint32_t, int64_t,
>>> uint64_t, intptr_t, uintptr_t,...
>>> see http://en.wikipedia.org/wiki/Stdint.h
>>>
>>> Yes, I know, Microsoft still do not have a compiler compliant with the
>>> C-standard published 10 years ago... but that can be worked around
>>> with a compat.h header to hide Microsoft's screw-ups, without
>>> 'uglyfying' the rest of the code.
>>>
>>>>
>>>> Kohei
>>>>
>>>> --
>>>> Kohei Yoshida, LibreOffice hacker, Calc
>>>> <kyoshida at novell.com>
>>>>
>>>> _______________________________________________
>>>> LibreOffice mailing list
>>>> LibreOffice at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/libreoffice
>>>>
>>>
>>
>