[libcamera-devel,v2,0/4] libcamera: Simplify range-based for loop counters
mbox series

Message ID 20210515040511.23294-1-laurent.pinchart@ideasonboard.com
Headers show
Series
  • libcamera: Simplify range-based for loop counters
Related show

Message

Laurent Pinchart May 15, 2021, 4:05 a.m. UTC 
Hello,

This patch series simplifies for loops that iterate over a range and
require a loop counter. v1 was sent as an RFC, and as it was positively
received, v2 graduates to a normal series.

The limitation of the range-based for loop is explained in patch 1/4,
which provides a new utils::enumerate() function to solve the problem.
The API is inspired by the C++23 views::enumerate proposal
(http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2164r1.pdf),
but implemented without the range library as that's a C++20 feature.

Patch 2/4 is an example of how code can be simplified. It also outlines
a limitation of the API. Due to the C++ lifetime rules, we can't safely
pass a temporary to utils::enumerate() as the temporary would be
destroyed once utils::enumerate() returns but before the for loop
completes (actually before it even starts), resulting in use-after-free
errors. The issue is explained in more details in
http://josuttis.com/download/std/D2012R0_fix_rangebasedfor_201029.pdf.
The ensure that such bugs can't be introduced by mistake, the
utils::enumerate() function takes an lvalue reference only. The drawback
is that a loop that iterates over a temporary must now stores that
temporary in a named variable before the loop. On a side note, C++20
would bring us a workaround to declare that variable within the loop
statement. If anyone can think of a clever idea to fix this problem, I'm
all ears. One option may be to store a copy of the iterable in the
adapter when utils::enumerate() is given an rvalue, possibly with move
semantics, but it may still be costly in some cases, I'm not sure how
compilers would optimize that.

Patch 3/4 provides an alternative API that uses an aggregate with named
fields instead of an std::pair with structured binding, with patch 4/4
adapting the example for 2/4 accordingly. There seems to be a preference
for the structured bindings, which means that 3/4 and 4/4 are likely to
be dropped. If they however end up being accepted after changes of mind,
I'll squash them with 1/4 and 2/4 respectively.

Laurent Pinchart (4):
  libcamera: utils: Add enumerate view for range-based for loops
  v4l2: Replace manual loop counters with utils::enumerate()
  libcamera: utils: enumerate: Use named fields for result
  v4l2: Adapt to utils::enumerate() usage of named fields

 include/libcamera/internal/utils.h | 92 ++++++++++++++++++++++++++++++
 src/libcamera/utils.cpp            | 30 ++++++++++
 src/v4l2/v4l2_compat_manager.cpp   | 18 +++---
 test/utils.cpp                     | 59 +++++++++++++++++++
 4 files changed, 190 insertions(+), 9 deletions(-)