239123 - exported functions in nsStringAPI.h should be frozen for 1.7 final

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Darin Fisher]

the exported functions in nsStringAPI.h should be frozen for 1.7 final.

Comment 1

[Darin Fisher]

It was suggested in bug 239303 that the string conversion routines define an
enum type name for the NS_ENCODING_* values.  Originally, I left this out
because I couldn't think of a good name for the type.  Lame, I admit.

Perhaps nsCStringEncoding is good choice?

Comment 2

[Brendan Eich [:brendan]]

Sounds like a fine name to me.  Some purists make the enum names use a prefix
derived from the type name, maybe with some contraction or acronym'ing. 
NS_CSTRING_ENCODING_* seems a bit overlong, though, and NS_CSTRENC_ is
cybercrud. So NS_ENCODING_* is fine by me, except there are lots of "encoders"
and "decoders" in our world, and we don't want to collide.

Your call, but NS_CSTRING_ENCODING_ is the sure way to avoid problems, even if a
bit overlong.

/be

Comment 3

[Darin Fisher]

I now think that we should also fixup the signatures of all the NS_C?String
methods to return nsresult (or maybe just PRBool) so that we have a way to
indicate OOM errors.  Sure, our current string implementation doesn't afford us
an easy way to detect these errors, but at least our API should have the ability
to convey it in the future.  For now, we can just lie and always return a
success code :-(

Comment 4

[Darin Fisher]

Attachment: v1 patch

OK, this patch does the following:

1) sets the visibility hidden attribute on all inline functions in
nsStringAPI.h.	this is necessary to allow embedders to use the nsAC?String
methods in a debug build without crashing on Linux.  the code as is requires
all methods on nsAC?String to be inlined, but that doesn't happen in a debug
build, so we need to make sure they are not visible to other modules, like
Gecko!

2) defines the nsCStringEncoding enum type w/ NS_ENCODING changed to
NS_CSTRING_ENCODING as discussed

3) adds BeginReading and EndReading methods to nsAC?String so embedders don't
have to resort to calling NS_C?StringGetData to access the raw buffer from a
nsAC?String instance.

4) in nsXPCOMGlue.cpp, replaced boolean xpcomGlueInit check with null checks of
the function pointers in xpcomFunctions.  this is basically reverting an
earlier change that i made during the 1.7 cycle.  i realized recently that
checking the individual functions is necessary in order to avoid crashes when a
component compiled against 1.7 is dropped into the components directory of for
example moz 1.6.

Comment 5

[Darin Fisher]

Comment on attachment 145720 [details] [diff] [review]
v1 patch

actually, this patch is incomplete.

Comment 6

[Darin Fisher]

Attachment: v1.1 patch

I had forgotten to update the comment in nsStringAPI.h that gives sample usage
of the NS_C?String functions.

Comment 7

[Darin Fisher]

Comment on attachment 145747 [details] [diff] [review]
v1.1 patch

for real this time :)

Comment 8

[Asa Dotzler [:asa]]

Comment on attachment 145747 [details] [diff] [review]
v1.1 patch

a=asa (on behalf of drivers) for checkin to 1.7

Comment 9

[Darin Fisher]

fixed-on-trunk for 1.7 final