| Message ID | 20210421072035.4188497-5-rosenp@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/8] clang-tidy: use auto | expand |
On 21/04/2021 09:20, Rosen Penev wrote: > The array has a nullptr and 0 member for some reason. Remove and convert > loop to a for range one. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > --- > utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ > 1 file changed, 15 insertions(+), 18 deletions(-) > > diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp > index cb3cb91f7..0359cf137 100644 > --- a/utils/common/v4l2-info.cpp > +++ b/utils/common/v4l2-info.cpp > @@ -3,6 +3,8 @@ > * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. > */ > > +#include <array> > + > #include <v4l2-info.h> > > static std::string num2s(unsigned num, bool is_hex = true) > @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) > return flags2s(flags, mbus_ycbcr_def); > } > > -static const flag_def selection_targets_def[] = { > - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, > - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, > - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, > - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, > - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, > - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, > - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, > - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, > - { 0, nullptr } The idea of having this sentinel is that this makes it easy to add new entries without having to update the array size. > +static constexpr std::array<flag_def, 8> selection_targets_def{ Something you need to do here, adding a new flag means updating the size. New flags are added regularly, so keeping that robust is a good idea IMHO. If it were possible to write: static constexpr std::array<flag_def> selection_targets_def{ i.e. without an explicit size, then this would make sense, but C++ doesn't allow this. And std::vector allocates the data on the heap, which is less efficient as well. Let's just keep using normal arrays in this case, they do the job just fine. Just because you have a hammer, it doesn't mean everything is now a nail :-) Regards, Hans > + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, > + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, > + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, > + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, > + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, > + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, > + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, > + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, > }; > > bool valid_seltarget_at_idx(unsigned i) > { > - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; > + return i < selection_targets_def.size(); > } > > unsigned seltarget_at_idx(unsigned i) > @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) > > std::string seltarget2s(__u32 target) > { > - int i = 0; > - > - while (selection_targets_def[i].str != nullptr) { > - if (selection_targets_def[i].flag == target) > - return selection_targets_def[i].str; > - i++; > - } > + for (const auto &def : selection_targets_def) > + if (def.flag == target) > + return def.str; > return std::string("Unknown (") + num2s(target) + ")"; > } > >
> On Apr 21, 2021, at 01:23, Hans Verkuil <hverkuil@xs4all.nl> wrote: > >> On 21/04/2021 09:20, Rosen Penev wrote: >> The array has a nullptr and 0 member for some reason. Remove and convert >> loop to a for range one. >> >> Signed-off-by: Rosen Penev <rosenp@gmail.com> >> --- >> utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ >> 1 file changed, 15 insertions(+), 18 deletions(-) >> >> diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp >> index cb3cb91f7..0359cf137 100644 >> --- a/utils/common/v4l2-info.cpp >> +++ b/utils/common/v4l2-info.cpp >> @@ -3,6 +3,8 @@ >> * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. >> */ >> >> +#include <array> >> + >> #include <v4l2-info.h> >> >> static std::string num2s(unsigned num, bool is_hex = true) >> @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) >> return flags2s(flags, mbus_ycbcr_def); >> } >> >> -static const flag_def selection_targets_def[] = { >> - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >> - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >> - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >> - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >> - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >> - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >> - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >> - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >> - { 0, nullptr } > > The idea of having this sentinel is that this makes it easy to add new > entries without having to update the array size. Not following this. I assume it’s some C feature. > >> +static constexpr std::array<flag_def, 8> selection_targets_def{ > > Something you need to do here, adding a new flag means updating the size. I assume this is a small issue. It’s an immediate compile error anyway. > > New flags are added regularly, so keeping that robust is a good idea IMHO. > > If it were possible to write: > > static constexpr std::array<flag_def> selection_targets_def{ > > i.e. without an explicit size, then this would make sense, but C++ > doesn't allow this. And std::vector allocates the data on the heap, > which is less efficient as well. But this is possible. With C++17 :). That would necessitate a minimum of GCC6 though. > > Let's just keep using normal arrays in this case, they do the job > just fine. Just because you have a hammer, it doesn't mean everything > is now a nail :-) > > Regards, > > Hans > >> + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >> + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >> + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >> + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >> + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >> + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >> + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >> + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >> }; >> >> bool valid_seltarget_at_idx(unsigned i) >> { >> - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; >> + return i < selection_targets_def.size(); >> } >> >> unsigned seltarget_at_idx(unsigned i) >> @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) >> >> std::string seltarget2s(__u32 target) >> { >> - int i = 0; >> - >> - while (selection_targets_def[i].str != nullptr) { >> - if (selection_targets_def[i].flag == target) >> - return selection_targets_def[i].str; >> - i++; >> - } >> + for (const auto &def : selection_targets_def) >> + if (def.flag == target) >> + return def.str; >> return std::string("Unknown (") + num2s(target) + ")"; >> } >> >> >
On 21/04/2021 11:19, Rosen Penev wrote: > >> On Apr 21, 2021, at 01:23, Hans Verkuil <hverkuil@xs4all.nl> wrote: >> >>> On 21/04/2021 09:20, Rosen Penev wrote: >>> The array has a nullptr and 0 member for some reason. Remove and convert >>> loop to a for range one. >>> >>> Signed-off-by: Rosen Penev <rosenp@gmail.com> >>> --- >>> utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ >>> 1 file changed, 15 insertions(+), 18 deletions(-) >>> >>> diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp >>> index cb3cb91f7..0359cf137 100644 >>> --- a/utils/common/v4l2-info.cpp >>> +++ b/utils/common/v4l2-info.cpp >>> @@ -3,6 +3,8 @@ >>> * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. >>> */ >>> >>> +#include <array> >>> + >>> #include <v4l2-info.h> >>> >>> static std::string num2s(unsigned num, bool is_hex = true) >>> @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) >>> return flags2s(flags, mbus_ycbcr_def); >>> } >>> >>> -static const flag_def selection_targets_def[] = { >>> - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >>> - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >>> - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >>> - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >>> - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >>> - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >>> - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >>> - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >>> - { 0, nullptr } >> >> The idea of having this sentinel is that this makes it easy to add new >> entries without having to update the array size. > Not following this. I assume it’s some C feature. Standard programming techique: https://en.wikipedia.org/wiki/Sentinel_value#:~:text=In%20computer%20programming%2C%20a%20sentinel,a%20loop%20or%20recursive%20algorithm. >> >>> +static constexpr std::array<flag_def, 8> selection_targets_def{ >> >> Something you need to do here, adding a new flag means updating the size. > I assume this is a small issue. It’s an immediate compile error anyway. Not if the size is larger that the number of initializers. That would leave zeroed elements at the end. >> >> New flags are added regularly, so keeping that robust is a good idea IMHO. >> >> If it were possible to write: >> >> static constexpr std::array<flag_def> selection_targets_def{ >> >> i.e. without an explicit size, then this would make sense, but C++ >> doesn't allow this. And std::vector allocates the data on the heap, >> which is less efficient as well. > But this is possible. With C++17 :). That would necessitate a minimum of GCC6 though. How is that done in C++17? I didn't find anything about that. Regards, Hans >> >> Let's just keep using normal arrays in this case, they do the job >> just fine. Just because you have a hammer, it doesn't mean everything >> is now a nail :-) >> >> Regards, >> >> Hans >> >>> + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >>> + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >>> + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >>> + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >>> + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >>> + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >>> + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >>> + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >>> }; >>> >>> bool valid_seltarget_at_idx(unsigned i) >>> { >>> - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; >>> + return i < selection_targets_def.size(); >>> } >>> >>> unsigned seltarget_at_idx(unsigned i) >>> @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) >>> >>> std::string seltarget2s(__u32 target) >>> { >>> - int i = 0; >>> - >>> - while (selection_targets_def[i].str != nullptr) { >>> - if (selection_targets_def[i].flag == target) >>> - return selection_targets_def[i].str; >>> - i++; >>> - } >>> + for (const auto &def : selection_targets_def) >>> + if (def.flag == target) >>> + return def.str; >>> return std::string("Unknown (") + num2s(target) + ")"; >>> } >>> >>> >>
> On Apr 21, 2021, at 02:33, Hans Verkuil <hverkuil@xs4all.nl> wrote: > >> On 21/04/2021 11:19, Rosen Penev wrote: >> >>>> On Apr 21, 2021, at 01:23, Hans Verkuil <hverkuil@xs4all.nl> wrote: >>>> >>>> On 21/04/2021 09:20, Rosen Penev wrote: >>>> The array has a nullptr and 0 member for some reason. Remove and convert >>>> loop to a for range one. >>>> >>>> Signed-off-by: Rosen Penev <rosenp@gmail.com> >>>> --- >>>> utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ >>>> 1 file changed, 15 insertions(+), 18 deletions(-) >>>> >>>> diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp >>>> index cb3cb91f7..0359cf137 100644 >>>> --- a/utils/common/v4l2-info.cpp >>>> +++ b/utils/common/v4l2-info.cpp >>>> @@ -3,6 +3,8 @@ >>>> * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. >>>> */ >>>> >>>> +#include <array> >>>> + >>>> #include <v4l2-info.h> >>>> >>>> static std::string num2s(unsigned num, bool is_hex = true) >>>> @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) >>>> return flags2s(flags, mbus_ycbcr_def); >>>> } >>>> >>>> -static const flag_def selection_targets_def[] = { >>>> - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >>>> - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >>>> - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >>>> - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >>>> - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >>>> - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >>>> - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >>>> - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >>>> - { 0, nullptr } >>> >>> The idea of having this sentinel is that this makes it easy to add new >>> entries without having to update the array size. >> Not following this. I assume it’s some C feature. > > Standard programming techique: > > https://en.wikipedia.org/wiki/Sentinel_value#:~:text=In%20computer%20programming%2C%20a%20sentinel,a%20loop%20or%20recursive%20algorithm. > >>> >>>> +static constexpr std::array<flag_def, 8> selection_targets_def{ >>> >>> Something you need to do here, adding a new flag means updating the size. >> I assume this is a small issue. It’s an immediate compile error anyway. > > Not if the size is larger that the number of initializers. That would leave > zeroed elements at the end. Ah yes that is a real problem. I’ve been bitten by it in the past. > >>> >>> New flags are added regularly, so keeping that robust is a good idea IMHO. >>> >>> If it were possible to write: >>> >>> static constexpr std::array<flag_def> selection_targets_def{ >>> >>> i.e. without an explicit size, then this would make sense, but C++ >>> doesn't allow this. And std::vector allocates the data on the heap, >>> which is less efficient as well. >> But this is possible. With C++17 :). That would necessitate a minimum of GCC6 though. > > How is that done in C++17? I didn't find anything about that. C++17 has class template argument deduction. It allows putting just std::array or std::vector or any other container really without the <>. Of course you have to allow it to deduce the arguments by putting them in the initializers. This is actually what I did in the previous cec-tuner patch. > > Regards, > > Hans > >>> >>> Let's just keep using normal arrays in this case, they do the job >>> just fine. Just because you have a hammer, it doesn't mean everything >>> is now a nail :-) >>> >>> Regards, >>> >>> Hans >>> >>>> + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >>>> + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >>>> + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >>>> + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >>>> + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >>>> + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >>>> + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >>>> + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >>>> }; >>>> >>>> bool valid_seltarget_at_idx(unsigned i) >>>> { >>>> - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; >>>> + return i < selection_targets_def.size(); >>>> } >>>> >>>> unsigned seltarget_at_idx(unsigned i) >>>> @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) >>>> >>>> std::string seltarget2s(__u32 target) >>>> { >>>> - int i = 0; >>>> - >>>> - while (selection_targets_def[i].str != nullptr) { >>>> - if (selection_targets_def[i].flag == target) >>>> - return selection_targets_def[i].str; >>>> - i++; >>>> - } >>>> + for (const auto &def : selection_targets_def) >>>> + if (def.flag == target) >>>> + return def.str; >>>> return std::string("Unknown (") + num2s(target) + ")"; >>>> } >>>> >>>> >>> >
Le mercredi 21 avril 2021 à 11:33 +0200, Hans Verkuil a écrit : > On 21/04/2021 11:19, Rosen Penev wrote: > > > > > On Apr 21, 2021, at 01:23, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > > > > > > On 21/04/2021 09:20, Rosen Penev wrote: > > > > The array has a nullptr and 0 member for some reason. Remove and convert > > > > loop to a for range one. > > > > > > > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > > > > --- > > > > utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ > > > > 1 file changed, 15 insertions(+), 18 deletions(-) > > > > > > > > diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp > > > > index cb3cb91f7..0359cf137 100644 > > > > --- a/utils/common/v4l2-info.cpp > > > > +++ b/utils/common/v4l2-info.cpp > > > > @@ -3,6 +3,8 @@ > > > > * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. > > > > */ > > > > > > > > +#include <array> > > > > + > > > > #include <v4l2-info.h> > > > > > > > > static std::string num2s(unsigned num, bool is_hex = true) > > > > @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) > > > > return flags2s(flags, mbus_ycbcr_def); > > > > } > > > > > > > > -static const flag_def selection_targets_def[] = { > > > > - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, > > > > - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, > > > > - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, > > > > - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, > > > > - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, > > > > - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, > > > > - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, > > > > - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, > > > > - { 0, nullptr } > > > > > > The idea of having this sentinel is that this makes it easy to add new > > > entries without having to update the array size. > > Not following this. I assume it’s some C feature. > > Standard programming techique: > > https://en.wikipedia.org/wiki/Sentinel_value#:~:text=In%20computer%20programming%2C%20a%20sentinel,a%20loop%20or%20recursive%20algorithm. > > > > > > > > +static constexpr std::array<flag_def, 8> selection_targets_def{ > > > > > > Something you need to do here, adding a new flag means updating the size. > > I assume this is a small issue. It’s an immediate compile error anyway. > > Not if the size is larger that the number of initializers. That would leave > zeroed elements at the end. > > > > > > > New flags are added regularly, so keeping that robust is a good idea IMHO. > > > > > > If it were possible to write: > > > > > > static constexpr std::array<flag_def> selection_targets_def{ > > > > > > i.e. without an explicit size, then this would make sense, but C++ > > > doesn't allow this. And std::vector allocates the data on the heap, > > > which is less efficient as well. > > But this is possible. With C++17 :). That would necessitate a minimum of GCC6 though. > > How is that done in C++17? I didn't find anything about that. I believe this is similar to this code: https://git.linuxtv.org/libcamera.git/tree/src/gstreamer/gstlibcamera-utils.cpp#n15 But there I only used the fact that you can iterate it like any other collection. This is a recent feature I got told (I'm quite a C++ newby, and I like static arrays without passing and updating a size). > > Regards, > > Hans > > > > > > > Let's just keep using normal arrays in this case, they do the job > > > just fine. Just because you have a hammer, it doesn't mean everything > > > is now a nail :-) > > > > > > Regards, > > > > > > Hans > > > > > > > + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, > > > > + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, > > > > + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, > > > > + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, > > > > + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, > > > > + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, > > > > + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, > > > > + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, > > > > }; > > > > > > > > bool valid_seltarget_at_idx(unsigned i) > > > > { > > > > - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; > > > > + return i < selection_targets_def.size(); > > > > } > > > > > > > > unsigned seltarget_at_idx(unsigned i) > > > > @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) > > > > > > > > std::string seltarget2s(__u32 target) > > > > { > > > > - int i = 0; > > > > - > > > > - while (selection_targets_def[i].str != nullptr) { > > > > - if (selection_targets_def[i].flag == target) > > > > - return selection_targets_def[i].str; > > > > - i++; > > > > - } > > > > + for (const auto &def : selection_targets_def) > > > > + if (def.flag == target) > > > > + return def.str; > > > > return std::string("Unknown (") + num2s(target) + ")"; > > > > } > > > > > > > > > > > >
diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp index cb3cb91f7..0359cf137 100644 --- a/utils/common/v4l2-info.cpp +++ b/utils/common/v4l2-info.cpp @@ -3,6 +3,8 @@ * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. */ +#include <array> + #include <v4l2-info.h> static std::string num2s(unsigned num, bool is_hex = true) @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) return flags2s(flags, mbus_ycbcr_def); } -static const flag_def selection_targets_def[] = { - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, - { 0, nullptr } +static constexpr std::array<flag_def, 8> selection_targets_def{ + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, }; bool valid_seltarget_at_idx(unsigned i) { - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; + return i < selection_targets_def.size(); } unsigned seltarget_at_idx(unsigned i) @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) std::string seltarget2s(__u32 target) { - int i = 0; - - while (selection_targets_def[i].str != nullptr) { - if (selection_targets_def[i].flag == target) - return selection_targets_def[i].str; - i++; - } + for (const auto &def : selection_targets_def) + if (def.flag == target) + return def.str; return std::string("Unknown (") + num2s(target) + ")"; }
The array has a nullptr and 0 member for some reason. Remove and convert loop to a for range one. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-)