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

Hi Norbert,

On Tue, 2011-01-04 at 01:57 -0600, Norbert Thiebaud wrote:
> 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)

Yes, technically you are right, and replacing ULONG with sal_uIntPtr
would be the safest choice.

My objection is mainly on the ground that using a type that is
suggestive of pointer value storage for values that are purely numerical
(such as the number format ID's) is, although safe from technical point
of view, not desirable.  And in most places in our code base where ULONG
is used, 32-bit integer range should be sufficient.  I'm sure there are
places where the real behavior of long is expected, so we need to go on
a case-by-case basis for this, but in the area where Soeren made his
changes, 32-bit range is more than sufficient.

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

Actually, I've been a proponent of these data types myself.  So I
personally would much prefer using these types over sal_Foo data types.
So, no objection there from me. :-)

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc
<kyoshida@novell.com>