Last Comment Bug 769533 - get rid of AppendwithConversion
: get rid of AppendwithConversion
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: String (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Makoto Kato [:m_kato]
:
Mentors:
Depends on:
Blocks: 113234
  Show dependency treegraph
 
Reported: 2012-06-28 20:40 PDT by Makoto Kato [:m_kato]
Modified: 2012-07-06 07:47 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
replace AppendWithConversion with others (9.48 KB, patch)
2012-06-28 20:40 PDT, Makoto Kato [:m_kato]
justin.lebar+bug: review+
Details | Diff | Splinter Review
get rid of AppendWithConversion (2.44 KB, patch)
2012-06-28 20:41 PDT, Makoto Kato [:m_kato]
justin.lebar+bug: review+
Details | Diff | Splinter Review

Description Makoto Kato [:m_kato] 2012-06-28 20:40:59 PDT
Created attachment 637775 [details] [diff] [review]
replace AppendWithConversion with others
Comment 1 Makoto Kato [:m_kato] 2012-06-28 20:41:27 PDT
Created attachment 637776 [details] [diff] [review]
get rid of AppendWithConversion
Comment 2 Justin Lebar (not reading bugmail) 2012-06-29 00:59:52 PDT
I can review this early next week if you like, Makoto.  My review load is probably lower than dbaron's.
Comment 3 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-06-29 11:36:19 PDT
That's fine.  I think the main interesting thing to investigate is whether the removal of the null-check is safe for all of the char*/PRUnichar* callers.
Comment 4 Justin Lebar (not reading bugmail) 2012-07-02 03:41:26 PDT
Comment on attachment 637775 [details] [diff] [review]
replace AppendWithConversion with others

># HG changeset patch
># Parent 2791940fb1ff454482ed91a76c9865c14b7b924e
>
>Part 1: replace AppendWithConversion with others
>
>dbaron: I think the main interesting thing to investigate is whether the
>removal of the null-check is safe for all of the char*/PRUnichar* callers.

I'm not sure what you mean -- AppendASCIItoUTF16 and LossyAppendUTF16toASCII both do null-checks on the pointers, so we should be good, right?

r=me pending dbaron's confirming that I'm not missing something here.
Comment 5 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-07-02 09:59:36 PDT
(In reply to Justin Lebar [:jlebar] from comment #4)
> I'm not sure what you mean -- AppendASCIItoUTF16 and LossyAppendUTF16toASCII
> both do null-checks on the pointers, so we should be good, right?

Oh, right.

Note You need to log in before you can comment on or make changes to this bug.