Bug 109792

Summary: Vector<T, inlineCapacity>::data() with size 0, returns null when inlineCapacity=0 and !null otherwise
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED LATER    
Severity: Normal CC: abarth, ahmad.saleem792, andersca, ap, benjamin, bfulgham, darin, mjs, rniwa, tonyg
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Eric Seidel (no email)
Reported 2013-02-14 01:22:18 PST
Vector<T, inlineCapacity>::data() with size 0, returns null when inlineCapacity=0 and !null otherwise This is inconsistent, and causes confusion when converting Vector<UChar> to Strings. See bug 109784
Attachments
Patch (5.19 KB, patch)
2013-02-15 21:47 PST, Benjamin Poulain 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2013-02-14 01:30:29 PST
This should be relatively easy to solve, we just have to be more careful to set m_buffer to 0 in the inlineCapacity case when size == 0.
Eric Seidel (no email)
Comment 2 2013-02-14 01:31:55 PST
Of course, once we fix this, we may need to fix bug 109784 in a different way for the threaded parser, but that's easy to do. :)
Maciej Stachowiak
Comment 3 2013-02-14 01:46:42 PST
What do you think the correct behavior should be, always null or never null?
Benjamin Poulain
Comment 4 2013-02-14 01:49:28 PST
(In reply to comment #3) > What do you think the correct behavior should be, always null or never null? I think undefined behavior could be okay, but always null would be more useful in practice.
Maciej Stachowiak
Comment 5 2013-02-14 02:02:43 PST
Looking at bug 109784, it seems to me that the right behavior for creating a String from an empty Vector is to get an empty string, not a null string. I am not sure if that has implications for this Vector issue. Originally the design intent of Vector was that you should not rely on data() when length() is 0. Making data() be always null is possible, but taking a quick look at the code, it would add extra branches to a number of VectorBase methods for the non-inline case (or else a branch in data() itself).
Benjamin Poulain
Comment 6 2013-02-14 02:10:12 PST
(In reply to comment #5) > Looking at bug 109784, it seems to me that the right behavior for creating a String from an empty Vector is to get an empty string, not a null string. I am not sure if that has implications for this Vector issue. I made a comment on that bug regarding that. > Originally the design intent of Vector was that you should not rely on data() when length() is 0. > > Making data() be always null is possible, but taking a quick look at the code, it would add extra branches to a number of VectorBase methods for the non-inline case (or else a branch in data() itself). Does making it null in debug sounds like an acceptable compromise to you? I would prefer it if people could not rely on that behavior, like in the case of Vector->String.
Benjamin Poulain
Comment 7 2013-02-15 21:47:41 PST
Created attachment 188691 [details] Patch
Benjamin Poulain
Comment 8 2013-02-15 21:49:04 PST
Quick stab at it, it probably does not compile. I think we may be able to make the change while reducing the number of branches.
Ahmad Saleem
Comment 9 2022-08-06 06:50:35 PDT
I can still see references to this in the Webkit Github source e.g.: https://github.com/WebKit/WebKit/blob/f407fbb287465a0ff68442eb7297862518e211cc/Source/WTF/wtf/Vector.h#L427 I think this patch need to rebased to land. Appreciate if someone is willing to work else I think we can mark this as "RESOLVED LATER". Thanks!
Ryosuke Niwa
Comment 10 2022-08-06 14:22:06 PDT
Later.
Note You need to log in before you can comment on or make changes to this bug.