[Lldb-commits] [lldb] r230046 - Add an OptionValueLanguage class

[Pavel Labath]

I have followed the discussion about this on lldb-dev a week ago (which did
not seem to have reached a definitive conclusion), but I must say I agree
with Greg's interpretation:

When I see a "const char *", I immediately assume it is null terminated
(unless accompanied by a size argument, or something like that). I think we
agree on that.

However, this is not the case for llvm::StringRef - it already contains the
length information, so I cease to care about null termination. In fact, the
entire purpose of the StringRef function (in my opinion) is to enable easy
processing of non-null terminated strings. You often need to take a
substring of a large string and pass it to some function as a parameter.
For example I can write "void foo(StringRef x) { x = x.trim(); bar(x); }".
Without StringRef I would either need to copy the string to ensure null
termination or modify the receiver to take a pointer+length combo. Now if
we demand that the StringRef contents be a null terminated string, we're
back at square one - we again need to copy, only this time we end up
passing the length parameter anyway. Then, I see no added value in using
StringRef over const char * (ok, you have the length precomputed, which can
save a couple of strlen()s, but that's not a big deal).


On 23 February 2015 at 16:01, Zachary Turner <zturner at google.com> wrote:

> No we don't. A const char* is not guaranteed to be null terminated either.
> Its up to you to make sure you pass it a null terminated string. Same
> applies here.
> On Mon, Feb 23, 2015 at 1:41 AM Pavel Labath <labath at google.com> wrote:
>
>>
>> On 20 February 2015 at 19:46, Enrico Granata <egranata at apple.com> wrote:
>>
>>> +Error
>>> +OptionValueLanguage::SetValueFromString (llvm::StringRef value,
>>> VarSetOperationType op)
>>> +{
>>> +    Error error;
>>> +    switch (op)
>>> +    {
>>> +    case eVarSetOperationClear:
>>> +        Clear();
>>> +        break;
>>> +
>>> +    case eVarSetOperationReplace:
>>> +    case eVarSetOperationAssign:
>>> +        {
>>> +            LanguageType new_type =
>>> LanguageRuntime::GetLanguageTypeFromString(value.data());
>>>
>>
>> Hi,
>>
>> llvm::StringRef::data() is not guaranteed to return a null terminated
>> string. We would need to call value.str().c_str() here, or (even better)
>> fix the receiver to operate on llvm::StringRef as well.
>>
>> cheers,
>> pl
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150223/9d8c95ed/attachment.html>