Closed Bug 323376 Opened 17 years ago Closed 12 years ago
Text Formatter::ssprintf adds extra '\0' to result
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051111 Firefox/1.5 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051111 Firefox/1.5 nsTextFormatter::ssprintf is appending an extra null to the end of the string it assigns to the passed in nsAString. The following code: nsAutoString value; nsTextFormatter::ssprintf(value, NS_LITERAL_STRING("%d").get(), 1); printf("%d\n", value.Length()); Will print 2 for the length of the string rather than 1. This is because dosprintf() explicitly appends a null to the end of the output string, even when working with an nsAString. The patch I am including simply changes the StringStuff function to do nothing when passed a null string. Reproducible: Always Steps to Reproduce:
Hmm, looks like this patch got abandoned. (I just happened upon this bug). To the reporter of this bug: The way the system works, the person who writes a patch is supposed to ask for review (see http://www.mozilla.org/hacking/code-review-faq.html). (Click edit next to the patch, set review to ? and put in the email address of the person you're asking for review). To get an idea of who to ask, check http://www.mozilla.org/owners.html. In this case, I suppose I'd suggest email@example.com; although that's just a guess; they'll redirect you if you ask the wrong person.
Comment on attachment 208454 [details] [diff] [review] The patch Review requested.
Attachment #208454 - Flags: review?(darin)
Comment on attachment 208454 [details] [diff] [review] The patch I recommend returning early if !*sp. That way you avoid indenting the rest of the function which helps make the code more readable IMO. r=darin with that change
Attachment #208454 - Flags: review?(darin) → review+
mass reassigning to nobody.
Assignee: dougt → nobody
Carrying Darin Fisher's r+.
Comment on attachment 480082 [details] [diff] [review] Un-bitrotted patch + comment taken into account I just understood that I needed approval for this patch to be accepted in the 2.0 timeframe, so here's the request. The patch is relatively low-risk, it's been reviewed already, and it really is a two-liner. The mailnews crowd would be definitely enlightened to see this checked in, as we currently have workarounds for this, that call .Cut() onto the nsStrings returned by the function.
Attachment #480082 - Flags: approval2.0?
Comment on attachment 480082 [details] [diff] [review] Un-bitrotted patch + comment taken into account a=beltzner
Attachment #480082 - Flags: approval2.0? → approval2.0+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Comment on attachment 480082 [details] [diff] [review] Un-bitrotted patch + comment taken into account Requesting approval for the 1.9.2 branch as Thunderbird 3.2 is likely going to be based on 1.9.2 and we'll need that fix in as well for the patches to work properly.
Attachment #480082 - Flags: approval18.104.22.168?
Will the removal of the workarounds have to be coordinated with the landing of this?
Target Milestone: mozilla2.0b8 → mozilla2.0b7
We checked-in a patch without the workarounds for comm-central, so as long as this patch is checked-in on the 1.9.2 branch Thunderbird 3.2 will be based on, I think it's fine. But Standard8 probably knows better.
Comment on attachment 480082 [details] [diff] [review] Un-bitrotted patch + comment taken into account Approved for 22.214.171.124, a=dveditz for release-drivers
Attachment #480082 - Flags: approval126.96.36.199? → approval188.8.131.52+
Thanks for the approval, marking as checkin-needed.
You need to log in before you can comment on or make changes to this bug.