[libcamera-devel] [RFC PATCH v1 01/12] libcamera: base: utils: Use size_t for index in utils::enumerate()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Sep 2 10:28:04 CEST 2021
Hi Kieran,
On Thu, Sep 02, 2021 at 08:48:44AM +0100, Kieran Bingham wrote:
> On 02/09/2021 05:22, Laurent Pinchart wrote:
> > The index generated by utils::enumerate() is an iteration counter, which
> > should thus be positive. Use std::size_t instead of the different_type
> > of the container.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> No other changes required elsewhere?
As far as the compiler tells me :-)
> I guess that means the update is not particularly invasive, and the
> existing users will be fine.
>
> Given that looking at the recommended usage for this is with auto, I
> guess that means the type is automatically updated for those users:
>
> src/v4l2/v4l2_compat_manager.cpp:
> for (auto [index, camera] : utils::enumerate(cameras)) {
Yes. It makes a difference when using index though, is comparing it to a
signed value will now produce a warning (hence the changes in the test).
There are no other usage of the variable that the compiler is unhappy
about.
> So this looks fine.
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > ---
> > include/libcamera/base/utils.h | 4 ++--
> > test/utils.cpp | 10 +++++-----
> > 2 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h
> > index 52301254c2eb..2b761436a99f 100644
> > --- a/include/libcamera/base/utils.h
> > +++ b/include/libcamera/base/utils.h
> > @@ -246,7 +246,7 @@ private:
> >
> > public:
> > using difference_type = typename std::iterator_traits<Base>::difference_type;
> > - using value_type = std::pair<const difference_type, base_reference>;
> > + using value_type = std::pair<const std::size_t, base_reference>;
> > using pointer = value_type *;
> > using reference = value_type &;
> > using iterator_category = std::input_iterator_tag;
> > @@ -275,7 +275,7 @@ public:
> >
> > private:
> > Base current_;
> > - difference_type pos_;
> > + std::size_t pos_;
> > };
> >
> > template<typename Base>
> > diff --git a/test/utils.cpp b/test/utils.cpp
> > index d7f810e95e7a..d65467b5102c 100644
> > --- a/test/utils.cpp
> > +++ b/test/utils.cpp
> > @@ -77,8 +77,8 @@ protected:
> >
> > int testEnumerate()
> > {
> > - std::vector<int> integers{ 1, 2, 3, 4, 5 };
> > - int i = 0;
> > + std::vector<unsigned int> integers{ 1, 2, 3, 4, 5 };
> > + unsigned int i = 0;
> >
> > for (auto [index, value] : utils::enumerate(integers)) {
> > if (index != i || value != i + 1) {
> > @@ -93,12 +93,12 @@ protected:
> > ++i;
> > }
> >
> > - if (integers != std::vector<int>{ 0, 1, 2, 3, 4 }) {
> > + if (integers != std::vector<unsigned int>{ 0, 1, 2, 3, 4 }) {
> > cerr << "Failed to modify container in enumerated range loop" << endl;
> > return TestFail;
> > }
> >
> > - Span<const int> span{ integers };
> > + Span<const unsigned int> span{ integers };
> > i = 0;
> >
> > for (auto [index, value] : utils::enumerate(span)) {
> > @@ -112,7 +112,7 @@ protected:
> > ++i;
> > }
> >
> > - const int array[] = { 0, 2, 4, 6, 8 };
> > + const unsigned int array[] = { 0, 2, 4, 6, 8 };
> > i = 0;
> >
> > for (auto [index, value] : utils::enumerate(array)) {
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list