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

Norbert Thiebaud nthiebaud at gmail.com
Tue Jan 4 02:18:13 PST 2011


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
>>>
>>
>


More information about the LibreOffice mailing list