88747 - nsCRT::atoi should be removed or fixed

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

|nsCRT::atoi| (according to a compiler warning pointed out by timeless, and by
inspection) just recurses infinitely.  |::atoi| takes a |char*|.  So we should
probably just remove it, unless someone wants to write one for |PRUnichar*|...

Comment 1

[timeless]

Attachment: impl

Comment 2

[Scott Collins]

Mmm.  Prefer pre-increment.  Prefer |for| loops for cases like this.  Make sure
you satisfy the actual behavior of the ASCII |atoi|, which means skipping
whitespace and possibly recognizing |'+'|.  It may be easier just to convert to
ASCII can call the systems |atoi| on the result (in terms of catching all the
edge cases).  |aChar| is a parameter style name, for a local pointer to
characters, prefer, e.g., |cp|, |s|, |t|... etc.,

Comment 3

[timeless]

Attachment: prefer to do less work

Comment 4

[timeless]

ok, so my first patch was composed while i was sleeping.  I had ruled out 
strings because i convinced myself they weren't available.  I should have tried 
compiling it.

kandrot can you test this function? [nothing in the tree uses it]

Comment 5

[Scott Collins]

sr=scc

Comment 6

[David Bradley]

r=dbradley

Comment 7

[timeless]

fix checked in

Comment 8

[Jon Smirl]

This fix breaks the XPCONNECT_STANDALONE build. The h file for 
NS_ConvertUCS2toUTF8 is not included in the standalone build. 

It looks like NS_ConvertUCS2toUTF8 is defined in the obsolete string directory. 
Is it ok to use this function?

nsCRT.cpp
c:\standalone\mozilla\xpcom\ds\nsCRT.cpp(650) : error C2065: 'NS_ConvertUCS2toUT
F8' : undeclared identifier
c:\standalone\mozilla\xpcom\ds\nsCRT.cpp(650) : error C2228: left of '.get' must
 have class/struct/union type

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

> It looks like NS_ConvertUCS2toUTF8 is defined in the obsolete string directory. 
> Is it ok to use this function?

Well, it's not really obsolete yet.  It will be once the replacement has been
written.

Comment 10

[John Bandhauer]

Well, It doesn't look like anyone actually *calls* nsCRT::atoi...
http://lxr.mozilla.org/seamonkey/search?string=nsCRT%3A%3Aatoi

Other 16bit string stuff is broken for XPCOM_STANDALONE anyway. Let's just NOP 
nsCRT::atoi in XPCOM_STANDALONE builds and get on with our lives. We can add an 
#ifdef'd NS_NOTREACHED to make it easier for anyone who may end up calling this 
to figure out why the code is not doing the expected operation. 

Patch coming.

Comment 11

[John Bandhauer]

Attachment: unbust the XPCOM_STANDALONE build

Comment 12

[John Bandhauer]

Hey, this is bustage. How about some r= sr= and we get this in today?

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

r=dbaron

Comment 14

[Brendan Eich [:brendan]]

Good grief, how many #ifdefs do we need for XPCOM_STANDALONE?  Is there a bug on
cleaning up things so we eventually unify on a standalone xpcom that Mozilla can
use as-is?

sr=[e-mail] on the short-term patch.

/be

Comment 15

[timeless]

patch checked in fwiw bug 24515 track's Can't build XPCOM standalone -- sorry i 
contributed to the problem.

Comment 16

[John Bandhauer]

brendan: The thing that whacks XPCOM_STANDALONE so much is that it is defined to 
not have intl stuff, but the string code is laced with intl-isms. I'm short on 
simple solutions for this.

timeless: thanks for the checkin.

Comment 17

[David Gardiner]

Just for the record, this was actually a dup of 64054, which now that i've seen 
this bug, has also been marked fixed.

-dave