Re: svn commit: r1917082 - /apr/apr/trunk/buffer/apr_buffer.c

On 18 Apr 2024, at 15:57, Yann Ylavic <[email protected]> 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
—