[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