On 18 Apr 2024, at 15:57, Yann Ylavic <ylavic....@gmail.com> wrote:
>> could be:
>> APR_DECLARE(apr_status_t) apr_buffer_str_set(apr_buffer_t *buf,
>> char *str, apr_size_t len)
>> {
>> if (len == APR_BUFFER_STRING) {
>> len = str ? strlen(str) : 0;
>> }
>> if (len > APR_BUFFER_MAX) {
>> return APR_EINVAL;
>> }
>>
>> if (!str) {
>> buf->d.str = NULL;
>> buf->size = 0;
>> }
>> else {
>> buf->d.str = str;
>> buf->size = len;
>> }
>> buf->size |= ~APR_BUFFER_MAX;
>
> The above conditional is not even needed, just:
> buf->d.str = str;
> buf->size = len | ~APR_BUFFER_MAX;
> works too.
Hmmm…
In the current implementation apr_buffer_alloc() is the simple case (absolute
value of buf->size) and apr_buffer_len() is the complicated case.
Swapping them round as you’ve suggested makes a lot of sense, as we len far
more often than we alloc.
We still have the case that alloc is one larger than len, so the real limit is
SIZE_MAX/2-1. We can probably detect this with a simple mask, let me ponder.
All of this is a private implementation detail, your point this needs to be
emphasized is spot on.
The only thing public about apr_buffer_t is sizeof, nothing else.
Regards,
Graham
—
