Gerrit0 commented on code in PR #3859:
URL: https://github.com/apache/avro/pull/3859#discussion_r3567551317
##########
lang/c++/impl/BinaryDecoder.cc:
##########
@@ -19,12 +19,28 @@
#include "Decoder.hh"
#include "Exception.hh"
#include "Zigzag.hh"
+#include <cstdlib>
#include <memory>
namespace avro {
using std::make_shared;
+// Structural cap on the number of elements to skip in an array or map. Mirrors
+// the read-path limit in Generic.cc; AVRO_MAX_COLLECTION_ITEMS, when a
+// non-negative integer, overrides it.
+static int64_t maxCollectionStructural() {
+ const char *env = std::getenv("AVRO_MAX_COLLECTION_ITEMS");
Review Comment:
Yeah... I still really dislike this. The only environment variables prior to
your changes that I see in the Avro source are for unit tests/demo programs.
With this introduction, bug reports also need to include any environment
variables set for someone investigating the problem to be sure to be able to
reproduce.
I don't think a note in the code is sufficient for documentation if this is
going to stay. If Avro is going to be configurable via environment variables,
the documentation site needs a page [like Apache
Arrow](https://arrow.apache.org/docs/cpp/env_vars.html) where all variables are
enumerated.
##########
lang/c++/include/avro/Decoder.hh:
##########
@@ -169,6 +169,14 @@ public:
/// by the avro decoder. Similar set of problems occur if the Decoder
/// consumes more than what it should.
virtual void drain() = 0;
+
+ /// Returns the number of bytes that remain to be read from the underlying
+ /// stream, or a negative value when that count is not known (for example a
+ /// streaming source, or a decoder not backed by a byte stream). The
default
+ /// is "unknown"; byte-stream decoders override it so a length prefix or a
+ /// collection block count that exceeds the data actually available can be
+ /// rejected before allocating for it.
+ virtual int64_t bytesRemaining() const { return -1; }
Review Comment:
Looking at just the C++ implementation, `remainingBytes` does not appear on
main, so it's not a public API change here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
