Re: [dev] Re: Coding St{andards|yle}
Hi, much of what Herbert writes is beside my point I think that criticizing old rules that cause problems today by causing bigger, slower, limited code which introduces extra maintenance burdens and extra bugs from too tight types and signed-/ unsigned issues is right on topic. Is this not the topic we are talking about here? [...] to permit ints and longs for everything Who wants that? If you need fixed-width types you need them... but even then using typedefs or or packing them into classes is often better. Anyway, I've stated my points. And I have a lot of experience to back them up. With that said I'm getting out of this discussion, I just don't have enough time. --- Herbert Duerr du...@sun.com Registered Office: Sun Microsystems GmbH Sonnenallee 1, D-85551 Kirchheim-Heimstetten Commercial register of the Local Court of Munich: HRB 161028 Managing Directors: Thomas Schroeder Chairman of the Supervisory Board: Martin Haering - To unsubscribe, e-mail: dev-unsubscr...@openoffice.org For additional commands, e-mail: dev-h...@openoffice.org
Re: [dev] Re: Coding St{andards|yle}
Herbert Duerr wrote:
I think that criticizing old rules that cause problems today by
causing bigger, slower, limited code which introduces extra
maintenance burdens and extra bugs from too tight types and signed-/
unsigned issues is right on topic. Is this not the topic we are
talking about here?
Hi Herbert,
well, again just very slightly missing my point - see below ;)
[...] to permit ints and longs for everything
Who wants that? If you need fixed-width types you need them... but
even then using typedefs or or packing them into classes is often
better.
Anyway, I've stated my points. And I have a lot of experience to
back them up. With that said I'm getting out of this discussion, I
just don't have enough time.
Sorry to hear, maybe someone else is picking this up then - anyway:
so your suggestion is to use the native C ints by default, and
fixed-width types whenever there's a (recognized) need? And forcing
those fixed-width types into actual classes doing overflow,
underflow, and lossy conversion checking in non-pro builds?
If the answer is yes, then I think I have a decidedly bad feeling
about this - somewhat contrasting your experience mentioned above:
_everytime_ someone (preferrably exploiters) is putting integer math
to its limits inside OOo, there are bugs. Tons of them. Because
preventing overflows or truncation is non-trivial, even with fixed
ranges. Getting this right with only weak-relationed native int
sizes is even harder. So my (slightly amended) position on that
would be: iff the native ints are desirable (please scratch that
bigger, slower ... code from the first paragraph - that's truly a
red herring. For 99.9% of OOo's codebase), then let's make sure that
first off this dearly missed what every computer scientist should
know about integer math page is written. ;)
P.S.: case in point - X11 wire protocol:
struct _xPoint { INT16 x, y; };
X11 api:
struct XPoint { short x, y; };
Overflow handling: burdened on the app. Talk about transparency, and
POLS. XPoint being 'int' would have even been ok-ish, from an
abstraction perspective - 'short' is just a misguided attempt to
expose the INT16.
Cheers,
-- Thorsten
pgpWdGq2YUnJZ.pgp
Description: PGP signature
Re: [dev] Re: Coding St{andards|yle}
I wrote: [...] iff the native ints are desirable [...] then let's make sure that first off this dearly missed what every computer scientist should know about integer math page is written. ;) That's of course nonsense. This page needs to be written unconditionally. :) -- Thorsten pgpkvxOkUEPas.pgp Description: PGP signature
Re: [dev] Re: Coding St{andards|yle}
Herbert Duerr wrote: With their int equivalents. E.g. svx/inc/svx/svdpage.hxx: sal_uInt16 GetPageNum() const; could be replaced by svx/inc/svx/svdpage.hxx: int GetPageNum() const; And there are a gazillion other methods that can be found using grep sal_.*Int16 */inc/ Of course all these are no high priority else they would have been updated long ago... Hi Herbert, your suggestion shows the fundamental flaw I've pointed out earlier - that too much of the code makes implicit assumptions about the available int ranges. Just grep for 0x, 0xFFFE etc. and weep. Add to that performance implications - the Calc guys can likely tell the story much better than me, how 'simple' increases in row/column sizes affect overall speed. When I look through other high profile multi-platform sources such as gcc, the linux-kernel, etc. they have no problems to use native types for everything except for interfaces defined by their ABI or the API of the underlying platforms. The kernel code presumably gets enough coverage with different int sizes over the range of supported platforms, that such bad code gets weeded out; besides I'm rather certain the kernel code is overall much more defensive than OOo's. gcc is a different league, compiler are usually no target for exploits. Ultimately, most of OOo's internal state stems from external input, and since there's no designed firewalling e.g. around the binary filters, I guess we're better safe than sorry, and not permit this extra dose of platform dependency, for no apparent benefit. Cheers, -- Thorsten pgp3ZSDvoix3d.pgp Description: PGP signature
Re: [dev] Re: Coding St{andards|yle}
Hi Michael, On Thursday, 2010-03-11 18:16:03 +0100, Michael Stahl wrote: imho, fixed size types are mandatory only on I/O paths: network protocols, file formats, and such. Btw, as long as we include that *gasp* binfilter module we'll have to deal with our own legacy and conversions or casts will be necessary there. That may also hold true for FASTBOOL and such if we don't copy module tools to binfilter as well. But hey, latest then it is time to say goodbye to binfilter.. Eike -- OOo/SO Calc core developer. Number formatter stricken i18n transpositionizer. SunSign 0x87F8D412 : 2F58 5236 DB02 F335 8304 7D6C 65C9 F9B5 87F8 D412 OpenOffice.org Engineering at Sun: http://blogs.sun.com/GullFOSS Please don't send personal mail to the e...@sun.com account, which I use for mailing lists only and don't read from outside Sun. Use er...@sun.com Thanks. pgpXDaMdskGyu.pgp Description: PGP signature
Re: [dev] Re: Coding St{andards|yle}
Hi Thorsten, please see my answers below. I don't think the fixed-width types are a hot topic that should get that much attention. Sounding the alarm on the use of native types is not attention-worthy either. Especially since the same reasoning got us worse code then, extra maintenance burdens now and extra bugs from too tight types and bugs from unsigned/ signed-mixing issue. your suggestion shows the fundamental flaw I've pointed out earlier - that too much of the code makes implicit assumptions about the available int ranges. Just grep for 0x, 0xFFFE etc. and weep. The hardcoded range-checks were probably good enough then because of the hardcoded types implied they would be safe forever. Changing any of these signatures now is a lot of work with lots of pain, little gain. That's why it isn't high on anybodies priority list. But if you need fixed-width types you should use them. But my corollary is that if you don't need them you shouldn't be forced to use them. Native types make code more pleasant to read. Code that is written without hardcoded 16bit/32bit/etc. assumptions can and IMHO should use native types. Add to that performance implications - the Calc guys can likely tell the story much better than me, how 'simple' increases in row/column sizes affect overall speed. Scalability problems are a different story. They show the same whether allowing 64K instead of 32K or whether allowing 96K instead of 64K. Of course changing old assumptions about reasonable ranges is hard as we can see in the Calc's row and column limits, on Writer's paragraph sizes, the number of resource IDs, etc. Nevertheless these limits should not stay there forever. When I look through other high profile multi-platform sources such as gcc, the linux-kernel, etc. they have no problems to use native types for everything except for interfaces defined by their ABI or the API of the underlying platforms. The kernel code presumably gets enough coverage with different int sizes over the range of supported platforms, that such bad code gets weeded out; besides I'm rather certain the kernel code is overall much more defensive than OOo's. gcc is a different league, compiler are usually no target for exploits. Using non-native types doesn't make problems go away automatically. In my experience it is quite the opposite: With fixed-width types the ranges were often chosen extra-tight. E.g. the artificial limit of 32K points in a tools polygon or your issue #128078# where refcounters overflowed. This tightness often causes their unsigned flavor to be used. Mixtures of signed/unsigned types in calculations easily result in surprises, which is why unsigned types are not supported in Java. Ultimately, most of OOo's internal state stems from external input, and since there's no designed firewalling e.g. around the binary filters, I guess we're better safe than sorry, and not permit this extra dose of platform dependency, for no apparent benefit. See my historical examples on the use of sal_Int16, USHORT etc. Hard- coding the internal interfaces to fixed-width types then results in worse code today. If types with no implicit lengths were used, e.g. intcoord instead While I'm on the topic of exact-width integer types: Shouldn't we use the types from cstdint instead? Is there any value-add left in the sal_Int/USHORT/ULONG/etc. types, except as a marker that the code hasn't been touched for a while? Or should they get some value-add by e.g. in debug mode by them becoming smart classes? With range checks, signed-unsigned checks, cast-checks, etc.? Range checks should be simple. Maybe there is already a generic template library for this? I haven't seen one though. If I had to implement it I'd use type names close to the cstdint typenames and switch the namespace depending on debug-mode or product-mode. --- Herbert Duerr du...@sun.com Registered Office: Sun Microsystems GmbH Sonnenallee 1, D-85551 Kirchheim-Heimstetten Commercial register of the Local Court of Munich: HRB 161028 Managing Directors: Thomas Schroeder Chairman of the Supervisory Board: Martin Haering - To unsubscribe, e-mail: dev-unsubscr...@openoffice.org For additional commands, e-mail: dev-h...@openoffice.org
Re: [dev] Re: Coding St{andards|yle}
Philipp Lohmann wrote: On 3/12/10 2:50 PM, Herbert Duerr wrote: your suggestion shows the fundamental flaw I've pointed out earlier - that too much of the code makes implicit assumptions about the available int ranges. Just grep for 0x, 0xFFFE etc. and weep. The hardcoded range-checks were probably good enough then because of the hardcoded types implied they would be safe forever. Changing any of these signatures now is a lot of work with lots of pain, little gain. That's why it isn't high on anybodies priority list. +1 Hi Philipp, much of what Herbert writes is beside my point - it's not about changing existing code, but about defaults for writing new, or fixing old. It's not whether the current code using fixed size ints is of especially good quality, but whether it would be rather harmful, or rather positive, to permit ints and longs for everything. Ages ago there was a decision to use typedefs like USHORT, ULONG, with apparently a fixed range - those were superseded by the recommendation of the uno types, now in the current coding standards. I still consider this decision valid, as it eliminates a very subtle way of introducing bugs (that are even platform-dependent, something we try hard to avoid in above-the-vcl code) - without any necessary drawbacks except, err, readability. Or should they get some value-add by e.g. in debug mode by them becoming smart classes? With range checks, signed-unsigned checks, cast-checks, etc.? Range checks should be simple. Maybe there is already a generic template library for this? I haven't seen one though. If I had to implement it I'd use type names close to the cstdint typenames and switch the namespace depending on debug-mode or product-mode. +1 You have a point here (and also with the unsigned ints are evil statement). But you'll need (much of) that checking for production code, too, if you say don't need fixed int range, will check for overflow myself. It's just much easier do code static checks at strategic places, if you know exact ranges, and not only weak = relations between types. Yes, I think the C int type system sucks. To this whole thread I can only say: don't we have more pressing problems than to change perfectly valid code to other perfectly valid code which in the best case does the same as before. See above. I was reasoning about coding style. Cheers, -- Thorsten pgpMitIjBFUgz.pgp Description: PGP signature
Re: [dev] Re: Coding St{andards|yle}
Herbert Duerr wrote: And while you are at it also replace the countless methods that still use sal_uInt16 instead of int as return value... goodbye Win3.1! ;-) Replace those methods with what? ;) -- Thorsten pgpq2wmSShlac.pgp Description: PGP signature
Re: [dev] Re: Coding St{andards|yle}
And while you are at it also replace the countless methods that still use sal_uInt16 instead of int as return value... goodbye Win3.1! ;-) Replace those methods with what? ;) With their int equivalents. E.g. svx/inc/svx/svdpage.hxx: sal_uInt16 GetPageNum() const; could be replaced by svx/inc/svx/svdpage.hxx: int GetPageNum() const; And there are a gazillion other methods that can be found using grep sal_.*Int16 */inc/ Of course all these are no high priority else they would have been updated long ago... When I look through other high profile multi-platform sources such as gcc, the linux-kernel, etc. they have no problems to use native types for everything except for interfaces defined by their ABI or the API of the underlying platforms. --- Herbert Duerr du...@sun.com Registered Office: Sun Microsystems GmbH Sonnenallee 1, D-85551 Kirchheim-Heimstetten Commercial register of the Local Court of Munich: HRB 161028 Managing Directors: Thomas Schroeder Chairman of the Supervisory Board: Martin Haering - To unsubscribe, e-mail: dev-unsubscr...@openoffice.org For additional commands, e-mail: dev-h...@openoffice.org
