From f588a379c4bbb385220ed6f3b04dcd3cd1cc7cec Mon Sep 17 00:00:00 2001
From: Javier Lopez-Gomez <javier.lopez.gomez@cern.ch>
Date: Thu, 23 Mar 2023 13:03:42 +0100
Subject: [PATCH] [ntuple] Fix misuse of the `TVirtualCollectionProxy` iterator
 interface

---
 tree/ntuple/v7/inc/ROOT/RField.hxx            |  6 ++---
 tree/ntuple/v7/test/SimpleCollectionProxy.hxx | 22 ++++++++++++-------
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/tree/ntuple/v7/inc/ROOT/RField.hxx b/tree/ntuple/v7/inc/ROOT/RField.hxx
index 9a85368a94e..b4a915dc5de 100644
--- a/tree/ntuple/v7/inc/ROOT/RField.hxx
+++ b/tree/ntuple/v7/inc/ROOT/RField.hxx
@@ -475,11 +475,11 @@ private:
          RIterator(const RCollectionIterableOnce &owner) : fOwner(owner) {}
          RIterator(const RCollectionIterableOnce &owner, void *iter) : fOwner(owner), fIterator(iter)
          {
-            fElementPtr = fOwner.fIFuncs.fNext(&fIterator, &fOwner.fEnd);
+            fElementPtr = fOwner.fIFuncs.fNext(fIterator, fOwner.fEnd);
          }
          iterator operator++()
          {
-            fElementPtr = fOwner.fIFuncs.fNext(&fIterator, &fOwner.fEnd);
+            fElementPtr = fOwner.fIFuncs.fNext(fIterator, fOwner.fEnd);
             return *this;
          }
          pointer operator*() const { return fElementPtr; }
@@ -498,7 +498,7 @@ private:
       {
          fIFuncs.fCreateIterators(collection, &fBegin, &fEnd, proxy);
       }
-      ~RCollectionIterableOnce() { fIFuncs.fDeleteTwoIterators(&fBegin, &fEnd); }
+      ~RCollectionIterableOnce() { fIFuncs.fDeleteTwoIterators(fBegin, fEnd); }
 
       RIterator begin() { return RIterator(*this, fBegin); }
       RIterator end() { return RIterator(*this); }
diff --git a/tree/ntuple/v7/test/SimpleCollectionProxy.hxx b/tree/ntuple/v7/test/SimpleCollectionProxy.hxx
index 6c68fe937e9..25f6bced785 100644
--- a/tree/ntuple/v7/test/SimpleCollectionProxy.hxx
+++ b/tree/ntuple/v7/test/SimpleCollectionProxy.hxx
@@ -7,28 +7,34 @@ namespace {
 /// Simple collection proxy for `StructUsingCollectionProxy<T>`
 template <typename CollectionT>
 class SimpleCollectionProxy : public TVirtualCollectionProxy {
-   CollectionT *fObject = nullptr;
+   /// The internal representation of an iterator, which in this simple test only contains a pointer to an element
+   struct IteratorData {
+      typename CollectionT::ValueType *ptr;
+   };
 
    static void
    Func_CreateIterators(void *collection, void **begin_arena, void **end_arena, TVirtualCollectionProxy * /*proxy*/)
    {
-      static_assert(sizeof(void *) <= TVirtualCollectionProxy::fgIteratorArenaSize);
+      static_assert(sizeof(IteratorData) <= TVirtualCollectionProxy::fgIteratorArenaSize);
       auto &vec = static_cast<CollectionT *>(collection)->v;
-      *begin_arena = &(*vec.begin());
-      *end_arena = &(*vec.end());
+      static_cast<IteratorData *>(*begin_arena)->ptr = &(*vec.begin());
+      static_cast<IteratorData *>(*end_arena)->ptr = &(*vec.end());
    }
 
    static void *Func_Next(void *iter, const void *end)
    {
-      auto &_iter = *static_cast<typename CollectionT::ValueType **>(iter);
-      auto _end = *static_cast<typename CollectionT::ValueType *const *>(end);
-      if (_iter >= _end)
+      auto _iter = static_cast<IteratorData *>(iter);
+      auto _end = static_cast<const IteratorData *>(end);
+      if (_iter->ptr >= _end->ptr)
          return nullptr;
-      return _iter++;
+      return _iter->ptr++;
    }
 
    static void Func_DeleteTwoIterators(void * /*begin*/, void * /*end*/) {}
 
+private:
+   CollectionT *fObject = nullptr;
+
 public:
    SimpleCollectionProxy()
       : TVirtualCollectionProxy(TClass::GetClass(ROOT::Internal::GetDemangledTypeName(typeid(CollectionT)).c_str()))
-- 
GitLab