Created attachment 356849 [details] [diff] [review] Patch This can happen if you call toLowerCase() or toUpperCase() on an empty string; it might be possible in other situations as well. Enclosed is a patch for the known case. Michael, you should probably examine the other possible cases to verify other misbehavior isn't present.
Attachment #356849 - Flags: review?(daumling)
Comment on attachment 356849 [details] [diff] [review] Patch I would remove the assert. It may be legal to attempt to allocate zero bytes - there are tons of possibilities where this may happen, and you fix the problem later in a correct way anyway.
It is never legal to request a zero-byte allocation from MMGC; it will assert as well, further down. This just pushes the assertion further up the well. (That's why I requested you examine the other uses to avoid zero-byte allocations)
It is legal to request a zero-length dynamic string. There are numerous occasions where this may happen, for example if someone calls getFixedWidthString() on core->kEmptyString with a width of 16 bits. Since such a string may be created and later appended to, the fix that you entered (if # of bytes == 0, make it 1), makes perfect sense, but the assert does not. There will not be any assert in MMgc because of your fix. Even if I made sure that no other code is able to enter createDynamic() with a zero length, I do not think that it makes sense to add that much housekeeping code if the solution is to simply allocate 1 character.
Status: NEW → ASSIGNED
Ah, ok, I misunderstood. 0-len dynamic string == fine, 0-len gc allocation == not fine. I'll revisit the patch.
Created attachment 357000 [details] [diff] [review] Patch #2 Updated patch per Michael's suggestion.
Comment on attachment 357000 [details] [diff] [review] Patch #2 Go for it.
Attachment #357000 - Flags: review?(daumling) → review+
pushed to redux as changeset: 1298:69526ecaf558
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Resolved fixed engineering / work item that has been pushed. Setting status to verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.