Odd nul-termination in StatusArg?

[Mark Rotteveel]

I'm looking at StatusArg, method
void StatusVector::ImplStatusVector::putStrArg(unsigned startWith)

Specifically this part
```
if (*arg == isc_arg_cstring)
{
m_strings.reserve(m_strings.length() + arg[1] + 1);
m_strings.append(*ptr, arg[1]);
m_strings.append(1, '\0');
}
else
m_strings.append(*ptr, strlen(*ptr) + 1);
```

Given the Firebird string implementation already does nul-termination
internally, is there a specific point to the `m_strings.append(1, '\0')`
and including the nul-terminator of *ptr in the else branch?

It seems to me this can be simplified to:

```
if (*arg == isc_arg_cstring)
m_strings.append(*ptr, arg[1]);
else
m_strings.append(*ptr);
```

(also removed the reserve, as append does it internally).

Or is there something odd about needing nul-termination within the
string specifically here or something like that? (If so, I would have
expected a comment to that effect.)

Mark
--
Mark Rotteveel

[Dimitry Sibiryakov]

'Mark Rotteveel' via firebird-devel wrote 09.07.2025 16:24:
> Given the Firebird string implementation already does nul-termination
> internally, is there a specific point to the `m_strings.append(1, '\0')` and
> including the nul-terminator of *ptr in the else branch?

m_strings in this case is not a single string but a collection of strings
separated by <NUL> (and thus it is terminated by double null).
isc_arg_cstring is not guaranteed to be null-terminated so forcing of the
terminator is necessary to separate this (sub-)string from the next one.
reserve() is just an optimisation to avoid double expansion of storage.

--
WBR, SD.

[Mark Rotteveel]

Is that documented somewhere in the code and I skipped over it?

In any case, I'll just address my immediate concern (conversion
warning), and leave the rest of this oddness untouched.

That said, if m_strings is a collection, why isn't it something like
std::vector or something?

Mark
--
Mark Rotteveel

[Alex Peshkoff]

On 7/9/25 17:44, 'Mark Rotteveel' via firebird-devel wrote:
> On 09/07/2025 16:31, 'Dimitry Sibiryakov' via firebird-devel wrote:
>> 'Mark Rotteveel' via firebird-devel wrote 09.07.2025 16:24:
>>> Given the Firebird string implementation already does
>>> nul-termination internally, is there a specific point to the
>>> `m_strings.append(1, '\0')` and including the nul-terminator of *ptr
>>> in the else branch?
>>
>>    m_strings in this case is not a single string but a collection of
>> strings separated by <NUL> (and thus it is terminated by double null).
>>    isc_arg_cstring is not guaranteed to be null-terminated so forcing
>> of the terminator is necessary to separate this (sub-)string from the
>> next one.
>>    reserve() is just an optimisation to avoid double expansion of
>> storage.
>
> Is that documented somewhere in the code and I skipped over it?
>

Sooner of all not documented. At least when I was writing some status
vector parser for the first time I just looked at already existing parsers.

> That said, if m_strings is a collection, why isn't it something like
> std::vector or something?
>

Yes, certainly it was possible to use ObjectsArray<string>. But for a
set of typically short strings existing approach appeared to be more
efficient, first of all from RAM usage POV.