This is an automated email from the ASF dual-hosted git repository. swebb2066 pushed a commit to branch improve_bytebuffer_interface in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit 44fe7579a1fdc6a20186652fa03272af6e1497c4 Author: Stephen Webb <[email protected]> AuthorDate: Mon Apr 6 14:08:16 2026 +1000 Improve the ByteBuffer interface in the next ABI version --- src/main/cpp/aprsocket.cpp | 2 +- src/main/cpp/bytearrayinputstream.cpp | 2 +- src/main/cpp/bytearrayoutputstream.cpp | 2 +- src/main/cpp/bytebuffer.cpp | 17 +++- src/main/cpp/charsetdecoder.cpp | 113 ++++++++++++-------------- src/main/cpp/charsetencoder.cpp | 30 ++++--- src/main/cpp/fileinputstream.cpp | 2 +- src/main/cpp/fileoutputstream.cpp | 9 +- src/main/cpp/inputstreamreader.cpp | 9 +- src/main/cpp/socketoutputstream.cpp | 2 +- src/main/cpp/transcoder.cpp | 8 +- src/main/include/log4cxx/helpers/bytebuffer.h | 51 +++++++++++- src/test/cpp/autoconfiguretestcase.cpp | 3 +- 13 files changed, 147 insertions(+), 103 deletions(-) diff --git a/src/main/cpp/aprsocket.cpp b/src/main/cpp/aprsocket.cpp index a7064738..8115b7f7 100644 --- a/src/main/cpp/aprsocket.cpp +++ b/src/main/cpp/aprsocket.cpp @@ -131,7 +131,7 @@ size_t APRSocket::write(ByteBuffer& buf) apr_status_t status = apr_socket_send(_priv->socket, buf.current(), &written); #endif - buf.position(buf.position() + written); + buf.increment_position(written); totalWritten += written; if (status != APR_SUCCESS) diff --git a/src/main/cpp/bytearrayinputstream.cpp b/src/main/cpp/bytearrayinputstream.cpp index 152f92b6..b120c9db 100644 --- a/src/main/cpp/bytearrayinputstream.cpp +++ b/src/main/cpp/bytearrayinputstream.cpp @@ -66,7 +66,7 @@ int ByteArrayInputStream::read(ByteBuffer& dst) size_t bytesCopied = min(dst.remaining(), m_priv->buf.size() - m_priv->pos); std::memcpy(dst.current(), &m_priv->buf[m_priv->pos], bytesCopied); m_priv->pos += bytesCopied; - dst.position(dst.position() + bytesCopied); + dst.increment_position(bytesCopied); return (int)bytesCopied; } } diff --git a/src/main/cpp/bytearrayoutputstream.cpp b/src/main/cpp/bytearrayoutputstream.cpp index cfd45433..2eeab346 100644 --- a/src/main/cpp/bytearrayoutputstream.cpp +++ b/src/main/cpp/bytearrayoutputstream.cpp @@ -59,7 +59,7 @@ void ByteArrayOutputStream::write(ByteBuffer& buf, Pool& /* p */ ) m_priv->array.resize(sz + count); memcpy(&m_priv->array[sz], buf.current(), count); - buf.position(buf.limit()); + buf.increment_position(count); } std::vector<unsigned char> ByteArrayOutputStream::toByteArray() const diff --git a/src/main/cpp/bytebuffer.cpp b/src/main/cpp/bytebuffer.cpp index c78ea8e2..5d8fe7a9 100644 --- a/src/main/cpp/bytebuffer.cpp +++ b/src/main/cpp/bytebuffer.cpp @@ -48,12 +48,21 @@ void ByteBuffer::clear() m_priv->pos = 0; } +void ByteBuffer::carry() +{ + auto available = remaining(); + memmove(m_priv->base, current(), available); + m_priv->lim = m_priv->cap; + m_priv->pos = available; +} + void ByteBuffer::flip() { m_priv->lim = m_priv->pos; m_priv->pos = 0; } +#if LOG4CXX_ABI_VERSION <= 15 void ByteBuffer::position(size_t newPosition) { if (newPosition < m_priv->lim) @@ -80,7 +89,7 @@ void ByteBuffer::limit(size_t newLimit) m_priv->pos = m_priv->lim; } } - +#endif bool ByteBuffer::put(char byte) { @@ -128,3 +137,9 @@ size_t ByteBuffer::remaining() const return m_priv->lim - m_priv->pos; } +size_t ByteBuffer::increment_position(size_t byteCount) +{ + auto available = remaining(); + m_priv->pos += byteCount < available ? byteCount : available; + return remaining(); +} diff --git a/src/main/cpp/charsetdecoder.cpp b/src/main/cpp/charsetdecoder.cpp index ed79d440..4d002391 100644 --- a/src/main/cpp/charsetdecoder.cpp +++ b/src/main/cpp/charsetdecoder.cpp @@ -181,7 +181,7 @@ class MbstowcsCharsetDecoder : public CharsetDecoder if (*src == 0) { out.append(1, (logchar) 0); - in.position(in.position() + 1); + in.increment_position(1); } else { @@ -194,7 +194,7 @@ class MbstowcsCharsetDecoder : public CharsetDecoder BUFSIZE - 1, &mbstate); auto converted = src - cbuf; - in.position(in.position() + converted); + in.increment_position(converted); if (wCharCount == (size_t) -1) // Illegal byte sequence? { @@ -252,7 +252,7 @@ class TrivialCharsetDecoder : public CharsetDecoder const logchar* src = (const logchar*) (in.data() + in.position()); size_t count = remaining / sizeof(logchar); out.append(src, count); - in.position(in.position() + remaining); + in.increment_position(remaining); } return APR_SUCCESS; @@ -288,30 +288,28 @@ class UTF8CharsetDecoder : public CharsetDecoder virtual log4cxx_status_t decode(ByteBuffer& in, LogString& out) { - if (in.remaining() > 0) + auto availableByteCount = in.remaining(); + std::string tmp(in.current(), availableByteCount); + std::string::const_iterator iter = tmp.begin(); + + while (iter != tmp.end()) { - std::string tmp(in.current(), in.remaining()); - std::string::const_iterator iter = tmp.begin(); + unsigned int sv = Transcoder::decode(tmp, iter); - while (iter != tmp.end()) + if (sv == 0xFFFF) { - unsigned int sv = Transcoder::decode(tmp, iter); - - if (sv == 0xFFFF) - { - size_t offset = iter - tmp.begin(); - in.position(in.position() + offset); - return APR_BADARG; - } - else - { - Transcoder::encode(sv, out); - } + size_t offset = iter - tmp.begin(); + in.increment_position(offset); + return APR_BADARG; + } + else + { + Transcoder::encode(sv, out); } - - in.position(in.limit()); } + in.increment_position(availableByteCount); + return APR_SUCCESS; } @@ -340,20 +338,16 @@ class ISOLatinCharsetDecoder : public CharsetDecoder virtual log4cxx_status_t decode(ByteBuffer& in, LogString& out) { - if (in.remaining() > 0) - { - - const unsigned char* src = (unsigned char*) in.current(); - const unsigned char* srcEnd = src + in.remaining(); - - while (src < srcEnd) - { - unsigned int sv = *(src++); - Transcoder::encode(sv, out); - } + auto availableByteCount = in.remaining(); + auto src = in.current(); + auto srcEnd = src + availableByteCount; - in.position(in.limit()); + while (src < srcEnd) + { + auto sv = static_cast<unsigned int>(*src++); + Transcoder::encode(sv, out); } + in.increment_position(availableByteCount); return APR_SUCCESS; } @@ -388,30 +382,26 @@ class USASCIICharsetDecoder : public CharsetDecoder { log4cxx_status_t stat = APR_SUCCESS; - if (in.remaining() > 0) + auto availableByteCount = in.remaining(); + auto src = in.current(); + auto srcEnd = src + availableByteCount; + size_t byteCount = 0; + while (src < srcEnd) { + auto sv = static_cast<unsigned int>(*src++); - const unsigned char* src = (unsigned char*) in.current(); - const unsigned char* srcEnd = src + in.remaining(); - - while (src < srcEnd) + if (sv < 0x80) { - unsigned char sv = *src; - - if (sv < 0x80) - { - src++; - Transcoder::encode(sv, out); - } - else - { - stat = APR_BADARG; - break; - } + ++byteCount; + Transcoder::encode(sv, out); + } + else + { + stat = APR_BADARG; + break; } - - in.position(src - (const unsigned char*) in.data()); } + in.increment_position(byteCount); return stat; } @@ -435,27 +425,27 @@ class LocaleCharsetDecoder : public CharsetDecoder log4cxx_status_t decode(ByteBuffer& in, LogString& out) override { log4cxx_status_t result = APR_SUCCESS; - const char* p = in.current(); - size_t i = in.position(); - size_t remain = in.limit() - i; + auto p = in.current(); + auto availableByteCount = in.remaining(); + size_t byteCount = 0; #if !LOG4CXX_CHARSET_EBCDIC if (std::mbsinit(&this->state)) // ByteBuffer not partially decoded? { // Copy single byte characters - for (; 0 < remain && ((unsigned int) *p) < 0x80; --remain, ++i, p++) + for (; byteCount < availableByteCount && static_cast<unsigned int>(*p) < 0x80; ++byteCount, ++p) { out.append(1, *p); } } #endif // Decode characters that may be represented by multiple bytes - while (0 < remain) + while (byteCount < availableByteCount) { wchar_t ch = 0; - size_t n = std::mbrtowc(&ch, p, remain, &this->state); + size_t n = std::mbrtowc(&ch, p, availableByteCount - byteCount, &this->state); if (0 == n) // NULL encountered? { - ++i; + ++byteCount; break; } if (static_cast<std::size_t>(-1) == n) // decoding error? @@ -468,11 +458,10 @@ class LocaleCharsetDecoder : public CharsetDecoder break; } Transcoder::encode(static_cast<unsigned int>(ch), out); - remain -= n; - i += n; + byteCount += n; p += n; } - in.position(i); + in.increment_position(byteCount); return result; } diff --git a/src/main/cpp/charsetencoder.cpp b/src/main/cpp/charsetencoder.cpp index b08e277b..f153008d 100644 --- a/src/main/cpp/charsetencoder.cpp +++ b/src/main/cpp/charsetencoder.cpp @@ -112,7 +112,7 @@ class APRCharsetEncoder : public CharsetEncoder iter += ((initial_inbytes_left - inbytes_left) / sizeof(LogString::value_type)); } - out.position(out.position() + (initial_outbytes_left - outbytes_left)); + out.increment_position((initial_outbytes_left - outbytes_left)); return stat; } @@ -188,7 +188,7 @@ class WcstombsCharsetEncoder : public CharsetEncoder if (converted != (size_t) -1) { iter += chunkSize; - out.position(out.position() + converted); + out.increment_position(converted); break; } } @@ -196,7 +196,7 @@ class WcstombsCharsetEncoder : public CharsetEncoder else { iter += chunkSize; - out.position(out.position() + converted); + out.increment_position(converted); } } @@ -328,7 +328,7 @@ class TrivialCharsetEncoder : public CharsetEncoder (const char*) in.data() + (iter - in.begin()), requested * sizeof(logchar)); iter += requested; - out.position(out.position() + requested * sizeof(logchar)); + out.increment_position(requested * sizeof(logchar)); } return APR_SUCCESS; @@ -456,39 +456,43 @@ class LocaleCharsetEncoder : public CharsetEncoder } log4cxx_status_t encode ( const LogString& in - , LogString::const_iterator& iter + , LogString::const_iterator& nextCodePoint , ByteBuffer& out ) override { log4cxx_status_t result = APR_SUCCESS; #if !LOG4CXX_CHARSET_EBCDIC char* current = out.current(); - size_t remain = out.remaining(); + size_t availableByteCount = out.remaining(); + size_t byteCount = 0; if (std::mbsinit(&this->state)) // ByteBuffer not partially encoded? { // Copy single byte characters for (; - iter != in.end() && ((unsigned int) *iter) < 0x80 && 0 < remain; - iter++, remain--, current++) + nextCodePoint != in.end() && byteCount < availableByteCount && static_cast<unsigned int>(*nextCodePoint) < 0x80; + ++nextCodePoint, ++byteCount, ++current) { - *current = *iter; + *current = static_cast<char>(*nextCodePoint); } } #endif // Encode characters that may require multiple bytes - while (iter != in.end() && MB_CUR_MAX <= remain) + while (nextCodePoint != in.end() && byteCount < availableByteCount && MB_CUR_MAX <= (availableByteCount - byteCount)) { - auto ch = Transcoder::decode(in, iter); + LogString::const_iterator lastCodePoint = nextCodePoint; + auto ch = Transcoder::decode(in, nextCodePoint); + if (nextCodePoint == lastCodePoint) // invalid input sequence? + nextCodePoint = in.end(); auto n = std::wcrtomb(current, ch, &this->state); if (static_cast<std::size_t>(-1) == n) // not a valid wide character? { result = APR_BADARG; break; } - remain -= n; + byteCount += n; current += n; } - out.position(current - out.data()); + out.increment_position(byteCount); return result; } diff --git a/src/main/cpp/fileinputstream.cpp b/src/main/cpp/fileinputstream.cpp index 3a1ac091..2a1e892e 100644 --- a/src/main/cpp/fileinputstream.cpp +++ b/src/main/cpp/fileinputstream.cpp @@ -113,7 +113,7 @@ int FileInputStream::read(ByteBuffer& buf) throw IOException(stat); } - buf.position(buf.position() + bytesRead); + buf.increment_position(bytesRead); retval = (int)bytesRead; } diff --git a/src/main/cpp/fileoutputstream.cpp b/src/main/cpp/fileoutputstream.cpp index d75634f5..c0155f51 100644 --- a/src/main/cpp/fileoutputstream.cpp +++ b/src/main/cpp/fileoutputstream.cpp @@ -110,22 +110,17 @@ void FileOutputStream::write(ByteBuffer& buf, Pool& /* p */ ) } size_t nbytes = buf.remaining(); - size_t pos = buf.position(); - const char* data = buf.data(); - while (nbytes > 0) { apr_status_t stat = apr_file_write( - m_priv->fileptr, data + pos, &nbytes); + m_priv->fileptr, buf.current(), &nbytes); if (stat != APR_SUCCESS) { throw IOException(stat); } - pos += nbytes; - buf.position(pos); - nbytes = buf.remaining(); + nbytes = buf.increment_position(nbytes); } } diff --git a/src/main/cpp/inputstreamreader.cpp b/src/main/cpp/inputstreamreader.cpp index 43885c3d..c1818ab7 100644 --- a/src/main/cpp/inputstreamreader.cpp +++ b/src/main/cpp/inputstreamreader.cpp @@ -91,16 +91,11 @@ LogString InputStreamReader::read(Pool& p) if (buf.remaining() > 0) { - const size_t carry = buf.remaining(); - - if (carry == BUFSIZE) + if (buf.remaining() == BUFSIZE) { throw IOException(LOG4CXX_STR("Decoder made no progress")); } - - memmove(buf.data(), buf.current(), carry); - buf.clear(); - buf.position(carry); + buf.carry(); } else { diff --git a/src/main/cpp/socketoutputstream.cpp b/src/main/cpp/socketoutputstream.cpp index 3ddc78ec..6f2fffe9 100644 --- a/src/main/cpp/socketoutputstream.cpp +++ b/src/main/cpp/socketoutputstream.cpp @@ -75,7 +75,7 @@ void SocketOutputStream::write(ByteBuffer& buf, Pool& /* p */ ) m_priv->array.resize(sz + count); memcpy(&m_priv->array[sz], buf.current(), count); - buf.position(buf.limit()); + buf.increment_position(count); } } diff --git a/src/main/cpp/transcoder.cpp b/src/main/cpp/transcoder.cpp index 6a92de33..b50737fe 100644 --- a/src/main/cpp/transcoder.cpp +++ b/src/main/cpp/transcoder.cpp @@ -100,7 +100,7 @@ char* Transcoder::encodeUTF8(const LogString& src, Pool& p) void Transcoder::encodeUTF8(unsigned int sv, ByteBuffer& dst) { size_t bytes = encodeUTF8(sv, dst.current()); - dst.position(dst.position() + bytes); + dst.increment_position(bytes); } @@ -147,7 +147,7 @@ size_t Transcoder::encodeUTF8(unsigned int ch, char* dst) void Transcoder::encodeUTF16BE(unsigned int sv, ByteBuffer& dst) { size_t bytes = encodeUTF16BE(sv, dst.current()); - dst.position(dst.position() + bytes); + dst.increment_position(bytes); } @@ -177,7 +177,7 @@ size_t Transcoder::encodeUTF16BE(unsigned int ch, char* dst) void Transcoder::encodeUTF16LE(unsigned int sv, ByteBuffer& dst) { size_t bytes = encodeUTF16LE(sv, dst.current()); - dst.position(dst.position() + bytes); + dst.increment_position(bytes); } size_t Transcoder::encodeUTF16LE(unsigned int ch, char* dst) @@ -342,7 +342,7 @@ void Transcoder::decode(const std::string& src, LogString& dst) if (CharsetDecoder::isError(stat)) { dst.append(1, LOSSCHAR); - buf.position(buf.position() + 1); + buf.increment_position(1); } } diff --git a/src/main/include/log4cxx/helpers/bytebuffer.h b/src/main/include/log4cxx/helpers/bytebuffer.h index 31b5a474..74ee445b 100644 --- a/src/main/include/log4cxx/helpers/bytebuffer.h +++ b/src/main/include/log4cxx/helpers/bytebuffer.h @@ -28,7 +28,9 @@ namespace helpers { /** -* A byte buffer. +* An area of memory, a cursor into that memory and a count of remaining bytes. +* It does not own the memory, so does not allocate or free memory. +* The user must ensure the lifetime of the memory exceeds the lifeime of the class instance. */ class LOG4CXX_EXPORT ByteBuffer { @@ -36,23 +38,68 @@ class LOG4CXX_EXPORT ByteBuffer LOG4CXX_DECLARE_PRIVATE_MEMBER_PTR(ByteBufferPriv, m_priv) public: + /// A \c capacity sized area of memory at \c data. ByteBuffer(char* data, size_t capacity); ~ByteBuffer(); + /// Move the remaining bytes to the start of the memory area + /// and set the cursor to the end of the those bytes. + void carry(); + + /// Set the cursor to the start of the memory area + /// and use the capacity as the extent to which the cursor can advance. void clear(); + + /// Set the extent to which the cursor can advance, \c limit(), to the current cursor position + /// and move the cursor to the start of the memory area. void flip(); + /// The start of the memory area. char* data(); + /// The start of the memory area. const char* data() const; + + /// The memory at the cursor position. char* current(); + /// The memory at the cursor position. const char* current() const; + + /// The extent to which the cursor can advance + /// as an offset from the start of the memory area. + /// Intially this is the capacity of the buffer. size_t limit() const; + +#if LOG4CXX_ABI_VERSION <= 15 + /// Use \c newLimit as the extent to which the cursor can advance. + /// If \c newLimit exceeds the memory capacity, an exception is thrown. + /// If the current cursor is currently beyond \c newLimit + /// the cursor is changed to be at \c newLimit. void limit(size_t newLimit); +#endif + + /// The offset of the current cursor from the start of the memory area. size_t position() const; + + /// The number of bytes from the current cursor + /// until the cursor can no longer advance. size_t remaining() const; + +#if LOG4CXX_ABI_VERSION <= 15 + /// Use \c newPosition as the cursor position + /// providing it is less than the extent to which the cursor can advance, + /// otherwise set the cursor to the extent to which the cursor can advance. void position(size_t newPosition); +#endif + /// Advance the cursor by \c byteCount + /// if that does not exceed the extent to which the cursor can advance, + /// otherwise set the cursor to the extent to which the cursor can advance. + /// @returns The number of bytes until the cursor can no longer advance. + size_t increment_position(size_t byteCount); - bool put(char byte); + /// Store \c byteValue at the cursor position and advance the cursor position + /// unless the cursor cannot advance any further. + /// @returns true if \c byteValue was stored in the buffer. + bool put(char byteValue); private: diff --git a/src/test/cpp/autoconfiguretestcase.cpp b/src/test/cpp/autoconfiguretestcase.cpp index df21032f..26ed05cb 100644 --- a/src/test/cpp/autoconfiguretestcase.cpp +++ b/src/test/cpp/autoconfiguretestcase.cpp @@ -156,8 +156,7 @@ public: bbuf.put(*p); ++sz; } - bbuf.position(0); - bbuf.limit(sz); + bbuf.flip(); helpers::FileOutputStream of(m_configFile, true); of.write(bbuf, m_pool); of.flush(m_pool);
