Purpose of easy task 'Get rid of sal_uLong' ?

classic Classic list List threaded Threaded
17 messages Options
Lubos Lunak Lubos Lunak
Reply | Threaded
Open this post in threaded view
|

Purpose of easy task 'Get rid of sal_uLong' ?


 Hello,

 I've noticed [*] and the description of what to do makes me wonder what the
point of the task actually is.

 The link in the description seems to suggest that the changes may come from
OpenOffice later anyway, but if we want to do the changes ourselves in order
to clean up the code, then IMO we should just do the simple and logical thing
and not the OpenOffice description (where the tradition rules to do anything
but the simple and logical >:) ).

 Specifically, the simple and logical type for numbers happens to be 'int'.
Some kind of intptr type is usually only for ugly hacks, and bit-precise
types are mainly for marshalling. Is there any point in keeping the task as
it is or can I change it to 'use sal_uInt32 if the precise size is need, e.g.
for marshalling, use sal_uIntPtr if it is used for storing pointer value,
otherwise simply use int'? And, looking at this description, is there any
plan to get rid of these superfluous sal_xxx types eventually?

[*]
http://wiki.documentfoundation.org/Development/Easy_Hacks#Get_rid_of_sal_uLong

--
 Lubos Lunak
 [hidden email]
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

Hi Lubos,
On Mon, 28 Mar 2011 12:45:11 +0200
Lubos Lunak <[hidden email]> wrote:

>  Specifically, the simple and logical type for numbers happens to be
> 'int'. Some kind of intptr type is usually only for ugly hacks, and
> bit-precise types are mainly for marshalling. Is there any point in
> keeping the task as it is or can I change it to 'use sal_uInt32 if
> the precise size is need, e.g. for marshalling, use sal_uIntPtr if it
> is used for storing pointer value, otherwise simply use int'? And,
> looking at this description, is there any plan to get rid of these
> superfluous sal_xxx types eventually?

IHMO, our aim should be to have _one_ canonical set of numerical types
in LibreOffice, and not one for marshalling, one for other cases.
Using non sal_* numeric types is just asking for trouble in the long
run, because some day you will need to marshall that stuff somewhere.
And there is absolutely no hurt in using the sal_* typedefs(*).

Best Regards,

Bjoern

(*) Other than "native" numeric types, which might have interesting new
sizes on every new platform.
--
https://launchpad.net/~bjoern-michaelsen


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Lubos Lunak Lubos Lunak
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

On Monday 28 of March 2011, Bjoern Michaelsen wrote:

> Hi Lubos,
> On Mon, 28 Mar 2011 12:45:11 +0200
>
> Lubos Lunak <[hidden email]> wrote:
> >  Specifically, the simple and logical type for numbers happens to be
> > 'int'. Some kind of intptr type is usually only for ugly hacks, and
> > bit-precise types are mainly for marshalling. Is there any point in
> > keeping the task as it is or can I change it to 'use sal_uInt32 if
> > the precise size is need, e.g. for marshalling, use sal_uIntPtr if it
> > is used for storing pointer value, otherwise simply use int'? And,
> > looking at this description, is there any plan to get rid of these
> > superfluous sal_xxx types eventually?
>
> IHMO, our aim should be to have _one_ canonical set of numerical types
> in LibreOffice, and not one for marshalling, one for other cases.

 Why not? I generally just need a number and int fits that perfectly. I really
don't care how many bits it has as long as it works and don't need to wonder
if I should use sal_Int32, sal_UInt16, sal_Int64 or whatever.

> Using non sal_* numeric types is just asking for trouble in the long
> run, because some day you will need to marshall that stuff somewhere.

 How is that different from having different sal_whatever in the code and
marshalling with that? If size-specific types are kept only for marshalling,
then it's at least obvious what the marshalling format is.

> And there is absolutely no hurt in using the sal_* typedefs(*).

 It's more typing and it's foreign to anybody not used to the codebase. Sure,
that may sound like silly reasons, but it's still two more reasons than I can
come up with for the other way around. Besides, I have already seen a number
of commits changing BOOL/sal_Bool to bool, so at least some people seem to
see the value of abandoning the sal_ types where not necessary.

--
 Lubos Lunak
 [hidden email]
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Tor Lillqvist Tor Lillqvist
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

>  It's more typing and it's foreign to anybody not used to the codebase.

I fully agree with Lubos.

Seriously, it is not likely that LO will ever run on a platform where "int" is not at least 32 bits. So using sal_Int32 in cases where a plain int would work just as well is just silly.

--tml


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

In reply to this post by Lubos Lunak
Hi Lubos,

On Tue, 29 Mar 2011 15:11:44 +0200
Lubos Lunak <[hidden email]> wrote:

>  Why not? I generally just need a number and int fits that perfectly.
> I really don't care how many bits it has as long as it works and
> don't need to wonder if I should use sal_Int32, sal_UInt16, sal_Int64
> or whatever.

For example you will create a lot of needless work for people who want
to get the code warning-free (which is a valid goal, see Caolans commits
for example) because of comparisons between different types
(signed/unsigned or different ranges). Almost every numeric data you
have in the codebase has been marshalled once, or will be (it was
either loaded, will be saved or is available though the API). All these
points are friction points. And LO is not a small project and will be
ported to new platforms. And the next platform will not be different
that the previous ports, there will always be some trouble.

> > Using non sal_* numeric types is just asking for trouble in the long
> > run, because some day you will need to marshall that stuff
> > somewhere.
>
>  How is that different from having different sal_whatever in the code
> and marshalling with that?

sal_* Types are typedefs and can be changed as a whole or be defined
different on different (or new) platforms. That eases a lot of trouble.

>  It's more typing and it's foreign to anybody not used to the
> codebase. Sure, that may sound like silly reasons

Indeed. And its wrong too. As "unsigned int" is for example a lot longer
to type than sal_*. Also its really not something only LO does:

http://www.gtk.org/api/2.6/glib/glib-Basic-Types.html
http://doc.qt.nokia.com/4.7-snapshot/datastreamformat.html

Also, you are not making it easier for the newcomers at all: If he gets
a sal_* type from one source and a creatively selected other type from
another source, you are making life hard for him. If he gets the same
types there is no trouble for him.

> Besides, I have already seen a number of commits changing
> BOOL/sal_Bool to bool, so at least some people seem to see the value
> of abandoning the sal_ types where not necessary.

That is a Strawman: I was talking about numeric types and numeric
types only, which the bool/BOOL/sal_Bool/FASTBOOL madness is completely
different from. Also sal_Bool and bool map perfectly and the same way
on all platforms(*).

That is not at all the case for numeric types (for example "long").
Someone writing code on *nix might easily store a sal_uInt64 in a long
on 64-Bit and create havoc on Windows in interesting and hard to
reproduce ways.

Best Regards,

Bjoern


(*) unless viciously abused.
--
https://launchpad.net/~bjoern-michaelsen

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

signature.asc (501 bytes) Download Attachment
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

On Tue, Mar 29, 2011 at 3:05 PM, Bjoern Michaelsen
<[hidden email]> wrote:

> Hi Lubos,
>
> On Tue, 29 Mar 2011 15:11:44 +0200
> Lubos Lunak <[hidden email]> wrote:
>
>>  How is that different from having different sal_whatever in the code
>> and marshalling with that?
>
> sal_* Types are typedefs and can be changed as a whole or be defined
> different on different (or new) platforms. That eases a lot of trouble.
>

I don't think you can do that, sal_* types are rightfully used for
on-disk structure.
you cannot change the size of these type between 2 platform, or you
could not open a file saved on one platform in another.

I think using sal_* judiciously for object that cross the 'external'
frontier is useful.
not so much because of the guaranteed size (intNN_t uintNN_t does that
very well) but because it could
be used to attract the attention to the fact that that
variable/meber/parameter is somehow 'externalized' either via ABI or
serialization.

Using sal_Int32 (or for that mater sal_Int16) in for() loop is not
useful and even conter productive.
forcing the compiler to generate non-optimal code for no good reason.

>
> Indeed. And its wrong too. As "unsigned int" is for example a lot longer
> to type than sal_*. Also its really not something only LO does:
>
> http://www.gtk.org/api/2.6/glib/glib-Basic-Types.html
please note the class of different type described in this link.
it boils down -- for  the most part -- to "stdint.h was not in the
standard yet so we needed to
define type with fixed size to deal with cross platform exchange..."

> http://doc.qt.nokia.com/4.7-snapshot/datastreamformat.html
please not that this last one is 'datastream' i.e serialization.
indeed when you serialize something you can't have compiler dependent
behavior.
quote:"It is always best to cast integers to a Qt integer type, such
as qint16 or quint32, when reading and writing. This ensures that you
always know exactly what size integers you are reading and writing, no
matter what the underlying platform and architecture the application
happens to be running on."

>
> Also, you are not making it easier for the newcomers at all: If he gets
> a sal_* type from one source and a creatively selected other type from
> another source, [...]
I would not consider using the C99 standard as 'creatively selected
from other source'

>
>
> That is not at all the case for numeric types (for example "long").
> Someone writing code on *nix might easily store a sal_uInt64 in a long
> on 64-Bit and create havoc on Windows in interesting and hard to
> reproduce ways.

I do agree with you on the long problem. which boils down to LLP64 vs
LP64 data model
i.e once again essentially Microsoft vs the rest of the world.
http://en.wikipedia.org/wiki/64-bit#64-bit_data_models

But that argument fall flat when one realized that we have code like

typedef sal_uIntPtr    sal_uLong; /* Replaces type ULONG */
...
THAT is confusing and unexpected and already borked with regard to
your valid objection above.

To summarize my view on the matter:

* sal_* type are useful and need to be strictly preserved when dealing
with ABI and/or data-serialization
* sal_Long sal_uLong are evil and confusing and inherently
non-portable. using sal- prefix here gives a false sens of security
* for internal code, especially intra-function local variable 'int'
should be favored when '32 bits is enough'. no platform we support (or
will support, unless someone want to backport libo to Apple II or
something :-) ) has an int that is less than 32 bits long.
* for other case, where one care of one reason or another about the
exact size, then int8_t, uint8_t, int16_t, uint16_t, int32_t,
uint32_t, int64_t, uint64_t are standardized type that do the job just
fine.

None of the above should be construed as meaning that I advocate a
'type conversion campaign'. except for the specific case of sal_uLong,
which _is_ borked already, I don't thing that the benefit/pain ratio
is big enough to justify such a big effort... not to mention the merge
conflicts... Just like agreeing that trailing spaces are a Bad
Thing(tm) does not mean that we are rushing to run clean-up scripts...

And finally, sal_uLong is a problem that need to be resolved. But for
the rest, I just expressed my personal preference. I certainly won't
make that a recurring sticking issue. If we decide stick with sal_*
everywhere, I won't be _that_ upset about it :-)

Norbert
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

Hi Norbert,

On Tue, 29 Mar 2011 17:28:26 -0500
Norbert Thiebaud <[hidden email]>
wrote:

> But that argument fall flat when one realized that we have code like
>
> typedef sal_uIntPtr    sal_uLong; /* Replaces type ULONG */
> ...
> THAT is confusing and unexpected and already borked with regard to
> your valid objection above.

Well, lets not forget sal_uLong was born to die:
http://www.mail-archive.com/dev@.../msg15185.html
 
> To summarize my view on the matter:
>
> * sal_* type are useful and need to be strictly preserved when dealing
> with ABI and/or data-serialization
> * sal_Long sal_uLong are evil and confusing and inherently
> non-portable. using sal- prefix here gives a false sens of security

There is no sal_Long. You are right about the sal_ prefix for sal_uLong
though. Esp. for a type defined in tools/solar.h. Maybe we should rename
that one to tools_uLongBrokenType? As you see in the mail above, that
type really _should_ look scary (given the history of types that should
be long be dead in the project by now and really arent at all).

> * for internal code, especially intra-function local variable 'int'
> should be favored when '32 bits is enough'. no platform we support (or
> will support, unless someone want to backport libo to Apple II or
> something :-) ) has an int that is less than 32 bits long.

I still have some fear in me from all the places in sw and elsewhere,
where I saw stuff like 0xFFFF and intended use of integer overflow. But
maybe thats just paranoia.

> * for other case, where one care of one reason or another about the
> exact size, then int8_t, uint8_t, int16_t, uint16_t, int32_t,
> uint32_t, int64_t, uint64_t are standardized type that do the job just
> fine.

Maybe we should use those even in sal/types.h by now?
 

> None of the above should be construed as meaning that I advocate a
> 'type conversion campaign'. except for the specific case of sal_uLong,
> which _is_ borked already, I don't thing that the benefit/pain ratio
> is big enough to justify such a big effort... not to mention the merge
> conflicts... Just like agreeing that trailing spaces are a Bad
> Thing(tm) does not mean that we are rushing to run clean-up scripts...
>
> And finally, sal_uLong is a problem that need to be resolved. But for
> the rest, I just expressed my personal preference. I certainly won't
> make that a recurring sticking issue. If we decide stick with sal_*
> everywhere, I won't be _that_ upset about it :-)
Understood. We mostly agree here as I see sal_uLong not as a sal-type
(not from sal/types.h).

Best Regards,

Bjoern

--
https://launchpad.net/~bjoern-michaelsen

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

signature.asc (501 bytes) Download Attachment
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

On Tue, Mar 29, 2011 at 6:51 PM, Bjoern Michaelsen
<[hidden email]> wrote:
> Maybe we should use those even in sal/types.h by now?

sure, modulo a bit of work to teach windows about standards... as C99
is way to recent of a standard for MS to have implemented it... Poor
things, surely no sane person could imagine that they'd turn around on
a dime and add a pair of include file in their distribution in less
than 10 years and 4 major release of their C/C++ compiler. they simply
don't have the ressource to do that :
http://connect.microsoft.com/VisualStudio/feedback/details/345360/visual-c-should-support-c99

I think we could use <boost/cstdint.hpp> or
http://code.google.com/p/msinttypes/source/browse/trunk/stdint.h or
http://www.azillionmonkeys.com/qed/pstdint.h or write our own

>
> I still have some fear in me from all the places in sw and elsewhere,
> where I saw stuff like 0xFFFF and intended use of integer overflow. But
> maybe thats just paranoia.
>

I would argue that that practice is broken.
use instead  UINT[16/32/64/]_MAX (or SAL_MAXUINT[8/16/32/64] if you
_must_ use SAL_ :-) )

note that a quick grep in writer give 74 match for FFFF, about half of
them being clearly irrelevant to this discussion.
So I think that soothing your paranoia with regard to this particular
point should not be that hard to do :-)

Globally it is a bit harder to count as there are quite a bit of
noise, but there are indeed quite a few magic constant. It is almost
never ok to use actual numbers a in a source,
all these 0xFFFF to indicate 'not-positioned' or 'not-found', should
really be some kind of defined symbol.
For example, starmath/inc/dialog.hxx define
#define CATEGORY_NONE 0xFFFF
which is almost right, it should be
#define CATEGORY_NONE SAL_MAX_UINT16

Actually in general I'm indeed concerned with:
#define FOO nnnn
almost always that should be
#define FOO ((numeric_type_cast)nnnn)


Norbert
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Tor Lillqvist Tor Lillqvist
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

In reply to this post by Bjoern Michaelsen
> Also its really not something only LO does:
>
> http://www.gtk.org/api/2.6/glib/glib-Basic-Types.html 

Whoa, stop there.

I can assure you that C code written in the "GLib/GTK+ style" definitely does *not* use gint32 or guint32 all over the place, if that was what you were hinting at.

gint32 and guint32 are used only in very specific cases where the code explicitly wants to use 32 bits on all platforms in all cases, like marshalling to/from some serialized data structure.

Otherwise it's int or its equivalent gint that is used. And yes, many of these people do care a lot about warning-free code, for example compiling GIMP is not supposed to produce warnings, as far as I know.

gint64 and guint64 are used in places where it is known that 32 bits is not enough, even if it isn't specifically exactly 64 bits that is necessarily wanted. Yes, that is wrong: Perhaps, some day, when we have machines where 128-bit integral types are equally natural and fast as 64-bit ones are today, this will be seen as a bad decision, and what should have been used instead would be some "gint64orlonger" type.

(I guess I should point out that GLib and the GTK+ stack are portable only to platforms where int is at least 32 bits.)

And yes, many do feel that the typedefs gint, guint, gchar and guchar in GLib are a bit silly, as it is 100% equivalent to just use int, unsigned int, char or unsigned char instead. Others do like to use them just for visual consistency in the code. Myself, I don't care that much, I use whatever the surrounding code does, and if writing completely new GTK+ code (which I hardly ever do) I use just int and char and not gint or gchar.

(With "GLib/GTK+ style I mean the style used by people that have been involved in writing and designing the GTK+ stack themselves, a typical example would be GIMP.)

--tml


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

On Wed, Mar 30, 2011 at 1:47 AM, Tor Lillqvist <[hidden email]> wrote:
[...]
>and what should have been used instead would be some "gint64orlonger" type.
>
btw that already exist in stdint.h , although not very keystroke friendly:

/ 7.18.1.2 Minimum-width integer types
int_least8_t;
int_least16_t;
int_least32_t;
int_least64_t;
uint_least8_t;
uint_least16_t;
uint_least32_t;
uint_least64_t;

// 7.18.1.3 Fastest minimum-width integer types
int_fast8_t;
int_fast16_t;
int_fast32_t;
int_fast64_t;
uint_fast8_t;
uint_fast16_t;
uint_fast32_t;
uint_fast64_t;
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Tor Lillqvist Tor Lillqvist
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

> btw that already exist in stdint.h , although not very keystroke friendly:

Yes, I know.

But as I am sure you also know, MSVC 2008 doesn't have stdint.h.

And please, let's not have this thread deteriorate into juvenile Microsoft-bashing.

--tml


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

On Wed, Mar 30, 2011 at 2:01 AM, Tor Lillqvist <[hidden email]> wrote:
>> btw that already exist in stdint.h , although not very keystroke friendly:
>
> Yes, I know.
>
> But as I am sure you also know, MSVC 2008 doesn't have stdint.h.

that can be worked around fairly easily. adding a couple of header is
not the worse things we've had to do...

>
> And please, let's not have this thread deteriorate into juvenile Microsoft-bashing.

This is not Microsoft-bashing, this is 'dumb-compiler-vendor' bashing.
The fact that this happen to be Microsoft is just an unsurprising co-incidence.

Norbert
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Lubos Lunak Lubos Lunak
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

In reply to this post by Bjoern Michaelsen
On Tuesday 29 of March 2011, Bjoern Michaelsen wrote:

> Hi Lubos,
>
> On Tue, 29 Mar 2011 15:11:44 +0200
>
> Lubos Lunak <[hidden email]> wrote:
> >  Why not? I generally just need a number and int fits that perfectly.
> > I really don't care how many bits it has as long as it works and
> > don't need to wonder if I should use sal_Int32, sal_UInt16, sal_Int64
> > or whatever.
>
> For example you will create a lot of needless work for people who want
> to get the code warning-free (which is a valid goal, see Caolans commits
> for example) because of comparisons between different types
> (signed/unsigned or different ranges).

 I don't think there's any warning for comparing differently sized types of
the same signedness, as the smaller one is simply promoted to the larger one.
And, because of the trouble caused by promotion rules when comparing signed
vs unsigned, it's just not worth using unsigned for anything except bitwise
operations. In fact insisting on the wide range of different types leads to
more warnings, not the other way around - if everything that's simply a
number is int, there are no warnings. I also doubt that using differently
sized typed rigorously gains anything - types smaller than int are promoted
to int in pretty much any operation, so presumably no warnings there, and as
soon as there's an operation that involves two different types, it's probably
just not worth the trouble.

> Almost every numeric data you
> have in the codebase has been marshalled once, or will be (it was
> either loaded, will be saved or is available though the API).

 How does API have to do anything with marshalling? Do you mean UNO with that?
And what do you estimate is the ratio of code that does marshalling to the
rest of the code?

> sal_* Types are typedefs and can be changed as a whole or be defined
> different on different (or new) platforms. That eases a lot of trouble.

 If it's used for marshalling, then it can't be changed. Or, if it can, then
it doesn't matter if the data would be simply marshalled as int.

> >  It's more typing and it's foreign to anybody not used to the
> > codebase. Sure, that may sound like silly reasons
>
> Indeed. And its wrong too. As "unsigned int" is for example a lot longer
> to type than sal_*.

 Typing is not really the biggest problem of unsigned int, see above.

> Also its really not something only LO does:
>
> http://www.gtk.org/api/2.6/glib/glib-Basic-Types.html
> http://doc.qt.nokia.com/4.7-snapshot/datastreamformat.html

 The Qt page is about data marshalling. Normally Qt uses int wherever
possible.

> Also, you are not making it easier for the newcomers at all: If he gets
> a sal_* type from one source and a creatively selected other type from
> another source, you are making life hard for him. If he gets the same
> types there is no trouble for him.

 Are you sure you're not arguing for my way here? The only way to get the same
type is if the codebase normally used int. Now it's full of different types
(sal_* ones and standard ones, all mixed).

> That is not at all the case for numeric types (for example "long").
> Someone writing code on *nix might easily store a sal_uInt64 in a long
> on 64-Bit and create havoc on Windows in interesting and hard to
> reproduce ways.

 I was talking about the case where the value is "just a number". If it needs
an extended range for whatever reason then that's something different.

 And I'm also not arguing for converting everything right now this very
moment. But I don't see why we should have an easy task that moves the
situation in the wrong direction.

--
 Lubos Lunak
 [hidden email]
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

Hi Lubos,

On Wed, 30 Mar 2011 11:21:26 +0200
Lubos Lunak <[hidden email]> wrote:

>  How does API have to do anything with marshalling?

Language bindings.

> Do you mean UNO with that?

Yes.

> And what do you estimate is the ratio of code that does marshalling
> to the rest of the code?

In the applications (and that is the major code block) practically
everything, as everything has at least some UNO wrapper -- and UNO is
also used a lot internally.

>  If it's used for marshalling, then it can't be changed. Or, if it
> can, then it doesn't matter if the data would be simply marshalled as
> int.

Some sal_* => "int" conversion that works on all current platforms does
not have to on the next one coming around.

>  Are you sure you're not arguing for my way here? The only way to get
> the same type is if the codebase normally used int. Now it's full of
> different types (sal_* ones and standard ones, all mixed).

I think most code now uses sal_* as of now and less code uses the
standard ones. For example, searching for this stuff roughly a la
find clone -name "*.cxx"|xargs grep "unsigned int"|wc -l
in clone (excluding libs-extern):

                               sal_uInt32    unsigned int
clone without external              24005            1246
  hxx                                7560             256
  cxx                               16445             999

Thus my conclusion.
 
>  And I'm also not arguing for converting everything right now this
> very moment. But I don't see why we should have an easy task that
> moves the situation in the wrong direction.

Agreed. Changing stuff there will just create lots of useless merge
conflicts. And if we see Oracle doing something stupid with sal_uLong,
we should do our better solution after _merging_ that. The worst
situtaton however is: they walk on to greener pastures because
priorities changed this week and sal_uLong keeps hanging around.

Best Regards,

Bjoern

--
https://launchpad.net/~bjoern-michaelsen

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

signature.asc (501 bytes) Download Attachment
Lubos Lunak Lubos Lunak
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

On Wednesday 30 of March 2011, Bjoern Michaelsen wrote:
> Lubos Lunak <[hidden email]> wrote:
> >  If it's used for marshalling, then it can't be changed. Or, if it
> > can, then it doesn't matter if the data would be simply marshalled as
> > int.
>
> Some sal_* => "int" conversion that works on all current platforms does
> not have to on the next one coming around.

 Of course it does. Either you want marshalling that works everywhere, in
which case types with given size should be used, and then it's better to be
explicit about them, or you just want it to work in that specific place (one
setup) and then it's just the same type, whatever it is. What other option is
there?

> >  Are you sure you're not arguing for my way here? The only way to get
> > the same type is if the codebase normally used int. Now it's full of
> > different types (sal_* ones and standard ones, all mixed).
>
> I think most code now uses sal_* as of now and less code uses the
> standard ones. For example, searching for this stuff roughly a la
> find clone -name "*.cxx"|xargs grep "unsigned int"|wc -l
> in clone (excluding libs-extern):
>
>                                sal_uInt32    unsigned int
> clone without external              24005            1246
>   hxx                                7560             256
>   cxx                               16445             999

 I think we both have, albeit for different reasons, agreed that nobody really
wants to use 'unsigned int'. I can give you the numbers for sal_uInt64 too if
you want a "proof" that there's only minor usage of sal_ types. Grep
for '\bint\b' or '\blong\b' if you want to see that standard types usage is
significant.

> >  And I'm also not arguing for converting everything right now this
> > very moment. But I don't see why we should have an easy task that
> > moves the situation in the wrong direction.
>
> Agreed. Changing stuff there will just create lots of useless merge
> conflicts. And if we see Oracle doing something stupid with sal_uLong,
> we should do our better solution after _merging_ that. The worst
> situtaton however is: they walk on to greener pastures because
> priorities changed this week and sal_uLong keeps hanging around.

 If the conclusion is that the easy task is not worth the merging problems,
that's still in line with my claim that the easy task is wrong as it is. It
just needs removing completely in that case.

--
 Lubos Lunak
 [hidden email]
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

On Thu, 31 Mar 2011 14:32:48 +0200
Lubos Lunak <[hidden email]> wrote:

> > >  And I'm also not arguing for converting everything right now this
> > > very moment. But I don't see why we should have an easy task that
> > > moves the situation in the wrong direction.
> >
> > Agreed. Changing stuff there will just create lots of useless merge
> > conflicts. And if we see Oracle doing something stupid with
> > sal_uLong, we should do our better solution after _merging_ that.
> > The worst situtaton however is: they walk on to greener pastures
> > because priorities changed this week and sal_uLong keeps hanging
> > around.
>
>  If the conclusion is that the easy task is not worth the merging
> problems, that's still in line with my claim that the easy task is
> wrong as it is. It just needs removing completely in that case.

Its already gone, thanks Kendy!

Best,

Bjoern

--
https://launchpad.net/~bjoern-michaelsen


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: Purpose of easy task 'Get rid of sal_uLong' ?

In reply to this post by Lubos Lunak
On Thu, 31 Mar 2011 14:32:48 +0200
Lubos Lunak <[hidden email]> wrote:

>  I think we both have, albeit for different reasons, agreed that
> nobody really wants to use 'unsigned int'. I can give you the numbers
> for sal_uInt64 too if you want a "proof" that there's only minor
> usage of sal_ types. Grep for '\bint\b' or '\blong\b' if you want to
> see that standard types usage is significant.

I still count 16426 \bint\bs (including external and unsigneds) and
55452+16426=71878 \bsal_Int32\b/\bsal_uInt32\bs (cxx only). "Standard
type" usage is still insignificant and changing that is a pointless
uphill battle.

Best Regards,

Bjoern

--
https://launchpad.net/~bjoern-michaelsen


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice