[PUSHED] Create OUString and OString number(*) methods.
Lubos Lunak
l.lunak at suse.cz
Mon Jan 21 09:05:30 PST 2013
On Monday 21 of January 2013, Stephan Bergmann wrote:
> On 01/18/2013 05:20 PM, Luboš Luňák (via Code Review) wrote:
> > https://gerrit.libreoffice.org/1625
>
> I must admit that I didn't review the later revisions of that patch
> while in progress at gerrit,
That's actually from me afterwards, when fixing a problem on 32bit (or was
that 64bit).
> but now the lines
>
> > assert( ll <= SAL_MAX_INT64 ); // valueOfInt64 may not be able to handle
> > the highest bit
>
> in the various number() function definitions make me wonder. That
> implies that clients of those functions need to ensure to call them with
> small-enough arguments. Is that what we want?
I think "small-enough" gives the wrong idea, it should be rather,
say, "not-insanely-large" arguments or so :). And despite the smiley, I'm
actually serious. This can trigger only when needing the topmost unsigned bit
of 64bit value, which is a 20 digit decimal number, and more importantly
those are values at the very limit of the supported integer range - there's
nothing larger than long long.
So even if there will be a legitimate case when unsigned long long will be
needed for the extra bit (which is pretty unlikely already), a lot of care
will be needed to handle it correctly (e.g. involving anything signed will be
potential trouble). That'll make this problem just a little bit more of
trouble.
> While the gotcha of printing a large unsigned value as a negative value
> thanks to calling rtl_ustr_valueOfInt64 internally can be a problem in
> some call sites, others might be fine with producing just some sort of
> informative value and won't mind generating negative output. If we
> would want to force the latter into using explicit casts to sal_Int64
> (in case they don't do already anyway), wouldn't it be better to make
> the relevant large unsigned overloads "= delete"?
That would mean blocking out all the values of the type, not just the
problematic ones. Given that a number of system typedefs are internally
unsigned integers despite all the signed vs unsigned trouble this causes,
usage of the type is much more likely than usage of a problematic value of
the type, so IMO it makes more sense to cater to realistic usage rathen than
handle more problematic but next to impossible scenarios.
--
Lubos Lunak
l.lunak at suse.cz
More information about the LibreOffice
mailing list