1380549 - nsStringBundle::FormatString seems to be doing an extra string copy for no reason

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

It's got a comment:

  // nsTextFormatter does not use the shared nsMemory allocator.
  // Instead it is required to free the memory it allocates using
  // nsTextFormatter::smprintf_free.  Let's instead use nsMemory based
  // allocation for the result that we give out and free the string
  // returned by smprintf ourselves!
  *aResult = NS_strdup(text);
  nsTextFormatter::smprintf_free(text);

But I'm fairly sure that at this point all the allocators involved are the same exact allocator, so this copy is pointless...

Should probably wait for bug 1380227 to land before messing with this.

Comment 1

[Mike Hommey [:glandium]]

Bug 1134923 removed NS_Alloc/NS_Realloc/NS_Free. There are leftovers because back then the standalone glue still mattered, but that's mostly gone as of bug 1332523. So we can actually remove those leftovers. Which then makes the whole point of NS_str*dup moot. I'd go as far as saying we should probably get rid of all the NS_str* functions, in favor of methods of the String classes (although those might actually be using the NS_str* functions under the hood at the moment).

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Remove unnecessary strdup in stringbundle code

We just have one allocator around now.

Comment 3

[Nicholas Nethercote [inactive]]

Looks like smprintf_free() can be removed altogether, and its uses replaced with free()? It already has a comment deprecating it. 

And a similar story for JS_smprintf_free() and js_free().

That would just leave PR_smprintf_free(), which is a harder one to remove thanks to it being NSPR.

Comment 4

[Mike Hommey [:glandium]]

Could you file a followup bug for comment 1/3?

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/59532b5342bc
Remove unnecessary strdup in stringbundle code.  r=glandium

Comment 6

[Boris Zbarsky [:bzbarsky]]

I filed bug 1381725, bug 1381726, and bug 1381727.

Thank you all for the review and suggestions!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/59532b5342bc