Re: [PATCH 2/2] strbuf: allow to use preallocated memory

[William Duclot <william.duclot@ensimag.grenoble-inp.fr>]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
> 
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>>> +/**
>>>> + * Allow the caller to give a pre-allocated piece of memory for the
>>>> strbuf
>>>> + * to use and indicate that the strbuf must use exclusively this buffer,
>>>> + * never realloc() it or allocate a new one. It means that the string
>>>> can
>>>> + * be manipulated but cannot overflow the pre-allocated buffer. The
>>>> + * pre-allocated buffer will never be freed.
>>>> + */
>>> 
>>> Perhaps say explicitly that although the allocated buffer has a fixed
>>> size, the string itself can grow as long as it does not overflow the
>>> buffer?
>>
>> That's what I meant by "the string can be manipulated but cannot overflow
>> the pre-allocated buffer". I'll try to reformulate
> 
> Maybe "the string can grow, but cannot overflow"?

Seems OK

>>>> @@ -91,6 +116,8 @@ extern void strbuf_release(struct strbuf *);
>>>>   * Detach the string from the strbuf and returns it; you now own the
>>>>   * storage the string occupies and it is your responsibility from then
>>>>   on
>>>>   * to release it with `free(3)` when you are done with it.
>>>> + * Must allocate a copy of the buffer in case of a preallocated/fixed
>>>> buffer.
>>>> + * Performance-critical operations have to be aware of this.
>>> 
>>> Better than just warn about performance, you can give the alternative.
>>
>> I'm not sure what you mean, I don't think there really is an alternative
>> for
>> detaching a string?
> 
> So, is the comment above saying "You're doomed, there's no way you can
> get good performance anyway"?
> 
> The alternative is just that you don't have to call strbuf_release since
> the caller can access the .buf field and is already the one responsible
> for freeing it when needed, and it's safe to just call strbuf_init() if
> one needs to re-initialize the stbuf structure.

Actually, if the user _needs_ to detach(), then yes he's doomed. Or more
realistically, he have to refactor so he'll don't need detach() (by being
sure the strbuf is pre-allocated by himself for example).
The strbuf_release() function is not the problem here, it does nothing
problematic for performances. The problem is strbuf_wrap_preallocated(),
but you're right I can add a comment so the user know when he don't have
to call it.