On 17 Apr 2024, at 07:52, Ruediger Pluem <rpl...@apache.org> wrote:
>> +APR_DECLARE(apr_buffer_t *) apr_buffer_str_make(apr_pool_t *pool,
>> + char *str, apr_ssize_t len)
>> +{
>> + apr_buffer_t *buf;
>> + apr_int64_t size;
>> + apr_size_t slen;
>> +
>> + if (!str) {
>> + str = NULL;
>> + size = 0;
>> + }
>> + if (APR_BUFFER_STRING == len && !str[0]) {
>> + size = len;
>> + }
>
> Is the above just a performance shortcut?
It is, not sure if it's overkill.
>> +APR_DECLARE(apr_buffer_t *) apr_buffer_cpy(apr_buffer_t *dst,
>> + const apr_buffer_t *src,
>> + apr_buffer_alloc alloc, void
>> *ctx)
>> +{
>> + if (!src) {
>> +
>> + dst->d.mem = NULL;
>> + dst->size = 0;
>> +
>> + }
>> + else if (!alloc) {
>> +
>> + dst->d.mem = src->d.mem;
>> + dst->size = src->size;
>> +
>> + }
>> + else {
>> +
>> + /* absolute value is size of mem buffer including optional
>> terminating zero */
>> + apr_int64_t size = src->size < 0 ? -src->size : src->size;
>> +
>> + void *mem = alloc(ctx, size);
>> + memcpy(mem, src->d.mem, size);
>> +
>> + dst->size = src->size;
>> + dst->d.mem = mem;
>
> Wouldn't it make sense to put the above 5 lines into a macro or a static
> inline function to reuse it
> here and in apr_buffer_arraydup?
My thoughts were to make all (most) of these macros, would that make sense?
>
>> +
>> + }
>> +
>> + return dst;
>> +}
>> +
>> +
>> +APR_DECLARE(int) apr_buffer_cmp(const apr_buffer_t *src,
>> + const apr_buffer_t *dst)
>> +{
>> + apr_size_t slen = apr_buffer_len(src);
>> + apr_size_t dlen = apr_buffer_len(dst);
>> +
>> + if (slen != dlen) {
>> + return slen < dlen ? -1 : 1;
>> + }
>> + else if (src->d.mem == dst->d.mem) {
>> + /* identical data or both NULL */
>> + return 0;
>> + }
>> + else if (!src->d.mem) {
>> + return -1;
>
> Can this case happen? We know that src->d.mem != dst->d.mem and if src->d.mem
> == NULL slen should be != dlen.
>
>> + }
>> + else if (!dst->d.mem) {
>> + return 1;
>
> See above.
Let me look.
My brain is utterly fried looking at this (and the LDAP code).
>> +APR_DECLARE(char *) apr_buffer_pstrncat(apr_pool_t *p, const apr_buffer_t
>> *buf,
>> + int nelts, const char *sep, int
>> flags,
>> + apr_size_t *nbytes)
>> +{
>> + const apr_buffer_t *src = buf;
>> + apr_size_t seplen = sep ? strlen(sep) : 0;
>> + apr_size_t size = 0;
>> +
>> + char *dst, *str;
>> +
>> + int i;
>> + for (i = 0; i < nelts; i++) {
>> +
>> + if (i > 0) {
>> + size += seplen;
>> + }
>> +
>> + if (src->size < 0) {
>> + size += (-src->size) - 1;
>> + }
>> + else {
>> + if (APR_BUFFER_NONE == flags) {
>> + size += src->size;
>> + }
>> + else if (APR_BUFFER_BASE64 == flags) {
>> + apr_size_t b64len;
>> +
>> + if (APR_SUCCESS != apr_encode_base64(NULL, src->d.mem,
>> src->size,
>> + APR_ENCODE_NONE,
>> &b64len)) {
>> + return NULL;
>> + }
>> + size += b64len - 1;
>> + }
>> + }
>> +
>> + src++;
>> + }
>> +
>> + if (nbytes) {
>> + *nbytes = size;
>> + }
>> +
>> + str = dst = apr_palloc(p, size + 1);
>> +
>> + src = buf;
>> +
>> + for (i = 0; i < nelts; i++) {
>> +
>> + if (i > 0 && sep) {
>> + strncpy(dst, sep, seplen);
>> + dst += seplen;
>> + }
>> +
>> + if (src->size < 0) {
>> + strncpy(dst, src->d.str, (-src->size) - 1);
>
> Why can't we use memcpy here? Do we assume that src->d.str contains a \0
> prior to (-src->size) - 1?
We can.
Regards,
Graham
--
