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

On 1/4/11, Soeren Moeller <soerenmoeller2001@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@gmail.com>:
> > On Mon, Jan 3, 2011 at 10:36 PM, Kohei Yoshida <kyoshida@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@novell.com>
> > >
> > > _______________________________________________
> > > LibreOffice mailing list
> > > LibreOffice@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/libreoffice