[PATCH] A new ADT: StringRefNulTerminated

Sean Silva silvas at purdue.edu
Wed Feb 20 20:54:36 PST 2013


On Wed, Feb 20, 2013 at 7:43 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Wed, Feb 20, 2013 at 4:34 PM, Sean Silva <silvas at purdue.edu> wrote:
>>
>> Overall, I think this is a good idea to have.
>>
>> Some comments about the patch:
>>
>> +    /// Construct a string ref from a string literal.
>> +    template<size_t LengthWithNul>
>> +    StringRefNulTerminated(const char (&Str)[LengthWithNul]):
>> +      StringRef(Str, LengthWithNul - 1) {}
>>
>> Is there a threat that this could end up being called by a non-string
>> literal? If so, then it should assert. I'm thinking of a scenario like
>
>
> As much fun as templated array ref ctors are - it's probably simpler just to
> take a const char*, use strlen always (you know it's null terminated - or at
> least the user is in the wrong if they pass you one that isn't) & let the
> compiler optimize away the strlen/inline the call when it's convenient to do
> so.
>
> That would catch cases of large buffers with small null terminated strings
> in them which the current patch doesn't handle correctly.
>
>>
>> const char Foo[1] = {'a'};
>> auto X = StringRefNulTerminated(Foo);
>
>
> Like any c-string API, that's just UB. Sure, I suppose we could use the
> array-ref style thing to detect & assert on this, might be handy - but we
> don't do it for other cstring APIs so I don't see any particular reason to
> do it on this one.

I just wanted to get this edge case on the table. Using a fixed size
char array for string manipulation in our codebase is so looked down
upon (banned, I would say; we have better and safer APIs) that
realistically it doesn't matter.

-- Sean Silva



More information about the llvm-commits mailing list