Re: svn commit: r1917266 - in /apr/apr/trunk: buffer/apr_buffer.c include/apr_buffer.h test/testbuffer.c

On 4/22/24 2:46 PM, [email protected] wrote:
> Author: minfrin
> Date: Mon Apr 22 12:46:37 2024
> New Revision: 1917266
> 
> URL: http://svn.apache.org/viewvc?rev=1917266&view=rev
> Log:
> apr_buffer: Redefine size to separate the unsigned size and the
> flag to indicate whether a zero terminated string or not.
> 
> Modified:
>     apr/apr/trunk/buffer/apr_buffer.c
>     apr/apr/trunk/include/apr_buffer.h
>     apr/apr/trunk/test/testbuffer.c
> 
> Modified: apr/apr/trunk/buffer/apr_buffer.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/buffer/apr_buffer.c?rev=1917266&r1=1917265&r2=1917266&view=diff
> ==============================================================================
> --- apr/apr/trunk/buffer/apr_buffer.c (original)
> +++ apr/apr/trunk/buffer/apr_buffer.c Mon Apr 22 12:46:37 2024

> @@ -322,7 +287,17 @@ APR_DECLARE(int) apr_buffer_ncmp(const a
>              return 1;
>          }

There was a proposal from Yann to simplify the above block to

    if (!src) {
        return dst ? 1 : 0;
    }
    if (!dst) {
        return -1;
    }

>          else {
> -            return apr_buffer_cmp(src, 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 {
> +                return memcmp(src->d.mem, dst->d.mem, slen);
> +            }
> +
>          }
>      }
>  }
                }
> @@ -379,19 +354,19 @@ APR_DECLARE(char *) apr_buffer_pstrncat(
>              strncpy(dst, sep, seplen);
>              dst += seplen;
>          }
> -        
> -        if (src->size < 0) {
> -            strncpy(dst, src->d.str, (apr_size_t)((-src->size) - 1));
> -            dst += (-src->size) - 1;
> +
> +        if (src->zero_terminated) {
> +            strncpy(dst, src->d.str, src->size);
> +            dst += src->size;
>          }

Can't we remove the above condition at all and do a

memcpy(dst, src->d.mem, src->size + src->zero_terminated);

below in the if (APR_BUFFER_NONE == flags) block?

>          else {
>              if (APR_BUFFER_NONE == flags) {
> -                memcpy(dst, src->d.mem, (apr_size_t)src->size);
> +                memcpy(dst, src->d.mem, src->size);
>              }
>              else if (APR_BUFFER_BASE64 == flags) {
>                  apr_size_t b64len;
>  
> -                if (APR_SUCCESS != apr_encode_base64(dst, src->d.mem, (apr_size_t)src->size,
> +                if (APR_SUCCESS != apr_encode_base64(dst, src->d.mem, src->size,
>                                                       APR_ENCODE_NONE, &b64len)) {
>                      return NULL;
>                  }
> 

Regards

Rüdiger