Re: [PATCH] gdb: fix missing null-character when using value_cstring

[Pedro Alves <pedro@palves.net>]

On 2023-04-06 2:20 p.m., Andrew Burgess via Gdb-patches wrote:
> Simon Marchi <simark@simark.ca> writes:
> 
>> On 4/3/23 17:49, Andrew Burgess via Gdb-patches wrote:

>>> Alternatively, if we change the requirements of value_cstring so that
>>> we require the passed length includes the trailing null, then we can
>>> assert that this is indeed the case within value_cstring.  Any
>>> incorrect uses in the future will be quickly spotted.
>>>
>>> So that's what I did, c_string_operation::evaluate is left unchanged,
>>> but every other use of value_cstring is adjusted with a '+ 1' so that
>>> we include the null within the length being passed.
>>
>> That sounds counterintuitive to me.  With an API of style pointer +
>> length, I don't expect the length to include the null terminator.  It
>> also unnecessarily forces the caller to have a null-terminated version
>> of the string, which may not always be the case (you might want to call
>> value_cstring on a subset of an existing string).
>>
>> I think that:
>>
>> struct value *
>> value_cstring (const char *ptr, ssize_t len, struct type *char_type)
>>
>> should take a length excluding the null terminator, but a null
>> terminator in the result (its job is to build a C string, and a C string
>> requires a null terminator at the end).
> 
> This is why writing comments is so important :)
> 
> I read it as "build a value* from this C-string", which is why I figured
> we can assume there will be a '\0' at the end.
> 
> Anyway, I don't really mind either way, just so long as we can get
> something that works!  I'll flip this around to the way you suggest and
> repost.
> 

Hmm, this rang a bell.  See this patch and following discussion:

 https://inbox.sourceware.org/gdb-patches/20210713153142.3957666-1-simon.marchi@efficios.com/

Pedro Alves