| Message ID | 20201211232319.32088-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 5cfbbcd20731a2160206cc1d935eac2c770376ae |
| Delegated to: | Laurent Pinchart |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Sat, Dec 12, 2020 at 01:23:19AM +0200, Laurent Pinchart wrote: > C++17 has a std::size() function that returns the size of a C-style > array. Use it instead of the custom ARRAY_SIZE macro. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > include/libcamera/internal/utils.h | 2 -- > src/ipa/raspberrypi/raspberrypi.cpp | 4 ++-- > src/libcamera/camera.cpp | 8 ++++---- > src/libcamera/log.cpp | 7 ++++--- > src/libcamera/utils.cpp | 5 ----- > src/libcamera/v4l2_videodevice.cpp | 6 +++--- > test/ipc/unixsocket.cpp | 6 +++--- > 7 files changed, 16 insertions(+), 22 deletions(-) > > diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h > index ebb2c4038e19..f08134afb5ba 100644 > --- a/include/libcamera/internal/utils.h > +++ b/include/libcamera/internal/utils.h > @@ -17,8 +17,6 @@ > #include <sys/time.h> > #include <vector> > > -#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0])) > - > #ifndef __DOXYGEN__ > > /* uClibc and uClibc-ng don't provide O_TMPFILE */ > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > index 801400214c3a..a7439badb7dc 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -6,6 +6,7 @@ > */ > > #include <algorithm> > +#include <array> > #include <fcntl.h> > #include <math.h> > #include <stdint.h> > @@ -27,7 +28,6 @@ > #include "libcamera/internal/buffer.h" > #include "libcamera/internal/camera_sensor.h" > #include "libcamera/internal/log.h" > -#include "libcamera/internal/utils.h" > > #include <linux/bcm2835-isp.h> > > @@ -1092,7 +1092,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls) > * Choose smallest cell size that won't exceed 63x48 cells. > */ > const int cellSizes[] = { 16, 32, 64, 128, 256 }; > - unsigned int numCells = ARRAY_SIZE(cellSizes); > + unsigned int numCells = std::size(cellSizes); > unsigned int i, w, h, cellSize; > for (i = 0; i < numCells; i++) { > cellSize = cellSizes[i]; > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 3579ba96b6ea..c1de1fee701a 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -7,6 +7,7 @@ > > #include <libcamera/camera.h> > > +#include <array> > #include <atomic> > #include <iomanip> > > @@ -17,7 +18,6 @@ > #include "libcamera/internal/log.h" > #include "libcamera/internal/pipeline_handler.h" > #include "libcamera/internal/thread.h" > -#include "libcamera/internal/utils.h" > > /** > * \file camera.h > @@ -393,7 +393,7 @@ int Camera::Private::isAccessAllowed(State state, bool allowDisconnected) const > if (currentState == state) > return 0; > > - ASSERT(static_cast<unsigned int>(state) < ARRAY_SIZE(camera_state_names)); > + ASSERT(static_cast<unsigned int>(state) < std::size(camera_state_names)); > > LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] > << " state trying operation requiring state " > @@ -412,8 +412,8 @@ int Camera::Private::isAccessAllowed(State low, State high, > if (currentState >= low && currentState <= high) > return 0; > > - ASSERT(static_cast<unsigned int>(low) < ARRAY_SIZE(camera_state_names) && > - static_cast<unsigned int>(high) < ARRAY_SIZE(camera_state_names)); > + ASSERT(static_cast<unsigned int>(low) < std::size(camera_state_names) && > + static_cast<unsigned int>(high) < std::size(camera_state_names)); > > LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] > << " state trying operation requiring state between " > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp > index 180eb97ba664..45c7c2d24652 100644 > --- a/src/libcamera/log.cpp > +++ b/src/libcamera/log.cpp > @@ -7,6 +7,7 @@ > > #include "libcamera/internal/log.h" > > +#include <array> > #if HAVE_BACKTRACE > #include <execinfo.h> > #endif > @@ -91,7 +92,7 @@ static const char *log_severity_name(LogSeverity severity) > "FATAL", > }; > > - if (static_cast<unsigned int>(severity) < ARRAY_SIZE(names)) > + if (static_cast<unsigned int>(severity) < std::size(names)) > return names[severity]; > else > return "UNKWN"; > @@ -406,7 +407,7 @@ void Logger::backtrace() > return; > > void *buffer[32]; > - int num_entries = ::backtrace(buffer, ARRAY_SIZE(buffer)); > + int num_entries = ::backtrace(buffer, std::size(buffer)); > char **strings = backtrace_symbols(buffer, num_entries); > if (!strings) > return; > @@ -620,7 +621,7 @@ LogSeverity Logger::parseLogLevel(const std::string &level) > severity = LogInvalid; > } else { > severity = LogInvalid; > - for (unsigned int i = 0; i < ARRAY_SIZE(names); ++i) { > + for (unsigned int i = 0; i < std::size(names); ++i) { > if (names[i] == level) { > severity = i; > break; > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp > index da85c9c24340..e90375ae115c 100644 > --- a/src/libcamera/utils.cpp > +++ b/src/libcamera/utils.cpp > @@ -31,11 +31,6 @@ namespace libcamera { > > namespace utils { > > -/** > - * \def ARRAY_SIZE(array) > - * \brief Determine the number of elements in the static array. > - */ > - > /** > * \brief Strip the directory prefix from the path > * \param[in] path The path to process > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index e2b582842a9b..baf683d6d985 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -7,6 +7,7 @@ > > #include "libcamera/internal/v4l2_videodevice.h" > > +#include <array> > #include <fcntl.h> > #include <iomanip> > #include <sstream> > @@ -26,7 +27,6 @@ > #include "libcamera/internal/log.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/media_object.h" > -#include "libcamera/internal/utils.h" > > /** > * \file v4l2_videodevice.h > @@ -860,7 +860,7 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set) > pix->num_planes = format->planesCount; > pix->field = V4L2_FIELD_NONE; > > - ASSERT(pix->num_planes <= ARRAY_SIZE(pix->plane_fmt)); > + ASSERT(pix->num_planes <= std::size(pix->plane_fmt)); > > for (unsigned int i = 0; i < pix->num_planes; ++i) { > pix->plane_fmt[i].bytesperline = format->planes[i].bpl; > @@ -1255,7 +1255,7 @@ std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) > buf.index = index; > buf.type = bufferType_; > buf.memory = V4L2_MEMORY_MMAP; > - buf.length = ARRAY_SIZE(v4l2Planes); > + buf.length = std::size(v4l2Planes); > buf.m.planes = v4l2Planes; > > int ret = ioctl(VIDIOC_QUERYBUF, &buf); > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp > index 19a1d7dd8a2d..80157b342795 100644 > --- a/test/ipc/unixsocket.cpp > +++ b/test/ipc/unixsocket.cpp > @@ -6,6 +6,7 @@ > */ > > #include <algorithm> > +#include <array> > #include <fcntl.h> > #include <iostream> > #include <stdlib.h> > @@ -19,7 +20,6 @@ > #include "libcamera/internal/ipc_unixsocket.h" > #include "libcamera/internal/thread.h" > #include "libcamera/internal/timer.h" > -#include "libcamera/internal/utils.h" > > #include "test.h" > > @@ -311,7 +311,7 @@ protected: > }; > int fds[2]; > > - for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { > + for (unsigned int i = 0; i < std::size(strings); i++) { > unsigned int len = strlen(strings[i]); > > fds[i] = open("/tmp", O_TMPFILE | O_RDWR, > @@ -333,7 +333,7 @@ protected: > if (ret) > return ret; > > - for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { > + for (unsigned int i = 0; i < std::size(strings); i++) { > unsigned int len = strlen(strings[i]); > char buf[len]; > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Laurent, On 12/12/20 4:53 AM, Laurent Pinchart wrote: > C++17 has a std::size() function that returns the size of a C-style > array. Use it instead of the custom ARRAY_SIZE macro. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Umang Jain <email@uajain.com> > --- > include/libcamera/internal/utils.h | 2 -- > src/ipa/raspberrypi/raspberrypi.cpp | 4 ++-- > src/libcamera/camera.cpp | 8 ++++---- > src/libcamera/log.cpp | 7 ++++--- > src/libcamera/utils.cpp | 5 ----- > src/libcamera/v4l2_videodevice.cpp | 6 +++--- > test/ipc/unixsocket.cpp | 6 +++--- > 7 files changed, 16 insertions(+), 22 deletions(-) > > diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h > index ebb2c4038e19..f08134afb5ba 100644 > --- a/include/libcamera/internal/utils.h > +++ b/include/libcamera/internal/utils.h > @@ -17,8 +17,6 @@ > #include <sys/time.h> > #include <vector> > > -#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0])) > - > #ifndef __DOXYGEN__ > > /* uClibc and uClibc-ng don't provide O_TMPFILE */ > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > index 801400214c3a..a7439badb7dc 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -6,6 +6,7 @@ > */ > > #include <algorithm> > +#include <array> > #include <fcntl.h> > #include <math.h> > #include <stdint.h> > @@ -27,7 +28,6 @@ > #include "libcamera/internal/buffer.h" > #include "libcamera/internal/camera_sensor.h" > #include "libcamera/internal/log.h" > -#include "libcamera/internal/utils.h" > > #include <linux/bcm2835-isp.h> > > @@ -1092,7 +1092,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls) > * Choose smallest cell size that won't exceed 63x48 cells. > */ > const int cellSizes[] = { 16, 32, 64, 128, 256 }; > - unsigned int numCells = ARRAY_SIZE(cellSizes); > + unsigned int numCells = std::size(cellSizes); > unsigned int i, w, h, cellSize; > for (i = 0; i < numCells; i++) { > cellSize = cellSizes[i]; > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 3579ba96b6ea..c1de1fee701a 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -7,6 +7,7 @@ > > #include <libcamera/camera.h> > > +#include <array> > #include <atomic> > #include <iomanip> > > @@ -17,7 +18,6 @@ > #include "libcamera/internal/log.h" > #include "libcamera/internal/pipeline_handler.h" > #include "libcamera/internal/thread.h" > -#include "libcamera/internal/utils.h" > > /** > * \file camera.h > @@ -393,7 +393,7 @@ int Camera::Private::isAccessAllowed(State state, bool allowDisconnected) const > if (currentState == state) > return 0; > > - ASSERT(static_cast<unsigned int>(state) < ARRAY_SIZE(camera_state_names)); > + ASSERT(static_cast<unsigned int>(state) < std::size(camera_state_names)); > > LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] > << " state trying operation requiring state " > @@ -412,8 +412,8 @@ int Camera::Private::isAccessAllowed(State low, State high, > if (currentState >= low && currentState <= high) > return 0; > > - ASSERT(static_cast<unsigned int>(low) < ARRAY_SIZE(camera_state_names) && > - static_cast<unsigned int>(high) < ARRAY_SIZE(camera_state_names)); > + ASSERT(static_cast<unsigned int>(low) < std::size(camera_state_names) && > + static_cast<unsigned int>(high) < std::size(camera_state_names)); > > LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] > << " state trying operation requiring state between " > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp > index 180eb97ba664..45c7c2d24652 100644 > --- a/src/libcamera/log.cpp > +++ b/src/libcamera/log.cpp > @@ -7,6 +7,7 @@ > > #include "libcamera/internal/log.h" > > +#include <array> > #if HAVE_BACKTRACE > #include <execinfo.h> > #endif > @@ -91,7 +92,7 @@ static const char *log_severity_name(LogSeverity severity) > "FATAL", > }; > > - if (static_cast<unsigned int>(severity) < ARRAY_SIZE(names)) > + if (static_cast<unsigned int>(severity) < std::size(names)) > return names[severity]; > else > return "UNKWN"; > @@ -406,7 +407,7 @@ void Logger::backtrace() > return; > > void *buffer[32]; > - int num_entries = ::backtrace(buffer, ARRAY_SIZE(buffer)); > + int num_entries = ::backtrace(buffer, std::size(buffer)); > char **strings = backtrace_symbols(buffer, num_entries); > if (!strings) > return; > @@ -620,7 +621,7 @@ LogSeverity Logger::parseLogLevel(const std::string &level) > severity = LogInvalid; > } else { > severity = LogInvalid; > - for (unsigned int i = 0; i < ARRAY_SIZE(names); ++i) { > + for (unsigned int i = 0; i < std::size(names); ++i) { > if (names[i] == level) { > severity = i; > break; > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp > index da85c9c24340..e90375ae115c 100644 > --- a/src/libcamera/utils.cpp > +++ b/src/libcamera/utils.cpp > @@ -31,11 +31,6 @@ namespace libcamera { > > namespace utils { > > -/** > - * \def ARRAY_SIZE(array) > - * \brief Determine the number of elements in the static array. > - */ > - > /** > * \brief Strip the directory prefix from the path > * \param[in] path The path to process > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index e2b582842a9b..baf683d6d985 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -7,6 +7,7 @@ > > #include "libcamera/internal/v4l2_videodevice.h" > > +#include <array> > #include <fcntl.h> > #include <iomanip> > #include <sstream> > @@ -26,7 +27,6 @@ > #include "libcamera/internal/log.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/media_object.h" > -#include "libcamera/internal/utils.h" > > /** > * \file v4l2_videodevice.h > @@ -860,7 +860,7 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set) > pix->num_planes = format->planesCount; > pix->field = V4L2_FIELD_NONE; > > - ASSERT(pix->num_planes <= ARRAY_SIZE(pix->plane_fmt)); > + ASSERT(pix->num_planes <= std::size(pix->plane_fmt)); > > for (unsigned int i = 0; i < pix->num_planes; ++i) { > pix->plane_fmt[i].bytesperline = format->planes[i].bpl; > @@ -1255,7 +1255,7 @@ std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) > buf.index = index; > buf.type = bufferType_; > buf.memory = V4L2_MEMORY_MMAP; > - buf.length = ARRAY_SIZE(v4l2Planes); > + buf.length = std::size(v4l2Planes); > buf.m.planes = v4l2Planes; > > int ret = ioctl(VIDIOC_QUERYBUF, &buf); > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp > index 19a1d7dd8a2d..80157b342795 100644 > --- a/test/ipc/unixsocket.cpp > +++ b/test/ipc/unixsocket.cpp > @@ -6,6 +6,7 @@ > */ > > #include <algorithm> > +#include <array> > #include <fcntl.h> > #include <iostream> > #include <stdlib.h> > @@ -19,7 +20,6 @@ > #include "libcamera/internal/ipc_unixsocket.h" > #include "libcamera/internal/thread.h" > #include "libcamera/internal/timer.h" > -#include "libcamera/internal/utils.h" > > #include "test.h" > > @@ -311,7 +311,7 @@ protected: > }; > int fds[2]; > > - for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { > + for (unsigned int i = 0; i < std::size(strings); i++) { > unsigned int len = strlen(strings[i]); > > fds[i] = open("/tmp", O_TMPFILE | O_RDWR, > @@ -333,7 +333,7 @@ protected: > if (ret) > return ret; > > - for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { > + for (unsigned int i = 0; i < std::size(strings); i++) { > unsigned int len = strlen(strings[i]); > char buf[len]; >
Hi Laurent On Sat, Dec 12, 2020 at 01:23:19AM +0200, Laurent Pinchart wrote: > C++17 has a std::size() function that returns the size of a C-style > array. Use it instead of the custom ARRAY_SIZE macro. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/internal/utils.h | 2 -- > src/ipa/raspberrypi/raspberrypi.cpp | 4 ++-- > src/libcamera/camera.cpp | 8 ++++---- > src/libcamera/log.cpp | 7 ++++--- > src/libcamera/utils.cpp | 5 ----- > src/libcamera/v4l2_videodevice.cpp | 6 +++--- > test/ipc/unixsocket.cpp | 6 +++--- > 7 files changed, 16 insertions(+), 22 deletions(-) > > diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h > index ebb2c4038e19..f08134afb5ba 100644 > --- a/include/libcamera/internal/utils.h > +++ b/include/libcamera/internal/utils.h > @@ -17,8 +17,6 @@ > #include <sys/time.h> > #include <vector> > > -#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0])) > - > #ifndef __DOXYGEN__ > > /* uClibc and uClibc-ng don't provide O_TMPFILE */ > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > index 801400214c3a..a7439badb7dc 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -6,6 +6,7 @@ > */ > > #include <algorithm> > +#include <array> > #include <fcntl.h> > #include <math.h> > #include <stdint.h> > @@ -27,7 +28,6 @@ > #include "libcamera/internal/buffer.h" > #include "libcamera/internal/camera_sensor.h" > #include "libcamera/internal/log.h" > -#include "libcamera/internal/utils.h" > > #include <linux/bcm2835-isp.h> > > @@ -1092,7 +1092,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls) > * Choose smallest cell size that won't exceed 63x48 cells. > */ > const int cellSizes[] = { 16, 32, 64, 128, 256 }; > - unsigned int numCells = ARRAY_SIZE(cellSizes); > + unsigned int numCells = std::size(cellSizes); I wonder if replacing cellSizes[] with a const std::array and use std::array::size() wouldn't be more C++-ish. The change itself is fine though Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > unsigned int i, w, h, cellSize; > for (i = 0; i < numCells; i++) { > cellSize = cellSizes[i]; > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 3579ba96b6ea..c1de1fee701a 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -7,6 +7,7 @@ > > #include <libcamera/camera.h> > > +#include <array> > #include <atomic> > #include <iomanip> > > @@ -17,7 +18,6 @@ > #include "libcamera/internal/log.h" > #include "libcamera/internal/pipeline_handler.h" > #include "libcamera/internal/thread.h" > -#include "libcamera/internal/utils.h" > > /** > * \file camera.h > @@ -393,7 +393,7 @@ int Camera::Private::isAccessAllowed(State state, bool allowDisconnected) const > if (currentState == state) > return 0; > > - ASSERT(static_cast<unsigned int>(state) < ARRAY_SIZE(camera_state_names)); > + ASSERT(static_cast<unsigned int>(state) < std::size(camera_state_names)); > > LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] > << " state trying operation requiring state " > @@ -412,8 +412,8 @@ int Camera::Private::isAccessAllowed(State low, State high, > if (currentState >= low && currentState <= high) > return 0; > > - ASSERT(static_cast<unsigned int>(low) < ARRAY_SIZE(camera_state_names) && > - static_cast<unsigned int>(high) < ARRAY_SIZE(camera_state_names)); > + ASSERT(static_cast<unsigned int>(low) < std::size(camera_state_names) && > + static_cast<unsigned int>(high) < std::size(camera_state_names)); > > LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] > << " state trying operation requiring state between " > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp > index 180eb97ba664..45c7c2d24652 100644 > --- a/src/libcamera/log.cpp > +++ b/src/libcamera/log.cpp > @@ -7,6 +7,7 @@ > > #include "libcamera/internal/log.h" > > +#include <array> > #if HAVE_BACKTRACE > #include <execinfo.h> > #endif > @@ -91,7 +92,7 @@ static const char *log_severity_name(LogSeverity severity) > "FATAL", > }; > > - if (static_cast<unsigned int>(severity) < ARRAY_SIZE(names)) > + if (static_cast<unsigned int>(severity) < std::size(names)) > return names[severity]; > else > return "UNKWN"; > @@ -406,7 +407,7 @@ void Logger::backtrace() > return; > > void *buffer[32]; > - int num_entries = ::backtrace(buffer, ARRAY_SIZE(buffer)); > + int num_entries = ::backtrace(buffer, std::size(buffer)); > char **strings = backtrace_symbols(buffer, num_entries); > if (!strings) > return; > @@ -620,7 +621,7 @@ LogSeverity Logger::parseLogLevel(const std::string &level) > severity = LogInvalid; > } else { > severity = LogInvalid; > - for (unsigned int i = 0; i < ARRAY_SIZE(names); ++i) { > + for (unsigned int i = 0; i < std::size(names); ++i) { > if (names[i] == level) { > severity = i; > break; > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp > index da85c9c24340..e90375ae115c 100644 > --- a/src/libcamera/utils.cpp > +++ b/src/libcamera/utils.cpp > @@ -31,11 +31,6 @@ namespace libcamera { > > namespace utils { > > -/** > - * \def ARRAY_SIZE(array) > - * \brief Determine the number of elements in the static array. > - */ > - > /** > * \brief Strip the directory prefix from the path > * \param[in] path The path to process > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index e2b582842a9b..baf683d6d985 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -7,6 +7,7 @@ > > #include "libcamera/internal/v4l2_videodevice.h" > > +#include <array> > #include <fcntl.h> > #include <iomanip> > #include <sstream> > @@ -26,7 +27,6 @@ > #include "libcamera/internal/log.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/media_object.h" > -#include "libcamera/internal/utils.h" > > /** > * \file v4l2_videodevice.h > @@ -860,7 +860,7 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set) > pix->num_planes = format->planesCount; > pix->field = V4L2_FIELD_NONE; > > - ASSERT(pix->num_planes <= ARRAY_SIZE(pix->plane_fmt)); > + ASSERT(pix->num_planes <= std::size(pix->plane_fmt)); > > for (unsigned int i = 0; i < pix->num_planes; ++i) { > pix->plane_fmt[i].bytesperline = format->planes[i].bpl; > @@ -1255,7 +1255,7 @@ std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) > buf.index = index; > buf.type = bufferType_; > buf.memory = V4L2_MEMORY_MMAP; > - buf.length = ARRAY_SIZE(v4l2Planes); > + buf.length = std::size(v4l2Planes); > buf.m.planes = v4l2Planes; > > int ret = ioctl(VIDIOC_QUERYBUF, &buf); > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp > index 19a1d7dd8a2d..80157b342795 100644 > --- a/test/ipc/unixsocket.cpp > +++ b/test/ipc/unixsocket.cpp > @@ -6,6 +6,7 @@ > */ > > #include <algorithm> > +#include <array> > #include <fcntl.h> > #include <iostream> > #include <stdlib.h> > @@ -19,7 +20,6 @@ > #include "libcamera/internal/ipc_unixsocket.h" > #include "libcamera/internal/thread.h" > #include "libcamera/internal/timer.h" > -#include "libcamera/internal/utils.h" > > #include "test.h" > > @@ -311,7 +311,7 @@ protected: > }; > int fds[2]; > > - for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { > + for (unsigned int i = 0; i < std::size(strings); i++) { > unsigned int len = strlen(strings[i]); > > fds[i] = open("/tmp", O_TMPFILE | O_RDWR, > @@ -333,7 +333,7 @@ protected: > if (ret) > return ret; > > - for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { > + for (unsigned int i = 0; i < std::size(strings); i++) { > unsigned int len = strlen(strings[i]); > char buf[len]; > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Mon, Dec 14, 2020 at 08:50:38AM +0100, Jacopo Mondi wrote: > On Sat, Dec 12, 2020 at 01:23:19AM +0200, Laurent Pinchart wrote: > > C++17 has a std::size() function that returns the size of a C-style > > array. Use it instead of the custom ARRAY_SIZE macro. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/internal/utils.h | 2 -- > > src/ipa/raspberrypi/raspberrypi.cpp | 4 ++-- > > src/libcamera/camera.cpp | 8 ++++---- > > src/libcamera/log.cpp | 7 ++++--- > > src/libcamera/utils.cpp | 5 ----- > > src/libcamera/v4l2_videodevice.cpp | 6 +++--- > > test/ipc/unixsocket.cpp | 6 +++--- > > 7 files changed, 16 insertions(+), 22 deletions(-) > > > > diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h > > index ebb2c4038e19..f08134afb5ba 100644 > > --- a/include/libcamera/internal/utils.h > > +++ b/include/libcamera/internal/utils.h > > @@ -17,8 +17,6 @@ > > #include <sys/time.h> > > #include <vector> > > > > -#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0])) > > - > > #ifndef __DOXYGEN__ > > > > /* uClibc and uClibc-ng don't provide O_TMPFILE */ > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > > index 801400214c3a..a7439badb7dc 100644 > > --- a/src/ipa/raspberrypi/raspberrypi.cpp > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > > @@ -6,6 +6,7 @@ > > */ > > > > #include <algorithm> > > +#include <array> > > #include <fcntl.h> > > #include <math.h> > > #include <stdint.h> > > @@ -27,7 +28,6 @@ > > #include "libcamera/internal/buffer.h" > > #include "libcamera/internal/camera_sensor.h" > > #include "libcamera/internal/log.h" > > -#include "libcamera/internal/utils.h" > > > > #include <linux/bcm2835-isp.h> > > > > @@ -1092,7 +1092,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls) > > * Choose smallest cell size that won't exceed 63x48 cells. > > */ > > const int cellSizes[] = { 16, 32, 64, 128, 256 }; > > - unsigned int numCells = ARRAY_SIZE(cellSizes); > > + unsigned int numCells = std::size(cellSizes); > > I wonder if replacing cellSizes[] with a const std::array and use > std::array::size() wouldn't be more C++-ish. The trouble with std::array() is that you need to declare the array size explicitly. This would become const std::array<int, 5> cellSizes{ 16, 32, 64, 128, 256 }; An experimental std::make_array() has been proposed (see https://en.cppreference.com/w/cpp/experimental/make_array) to overcome that limitation. It has been dropped in favour of std::to_array() (see https://en.cppreference.com/w/cpp/container/array/to_array) that has been merged in C++20. The code would then become const auto cellSizes = std::to_array<int>({ 16, 32, 64, 128, 256 }); We could possibly implement this function in the utils namespace (that would need to be double-checked though, as it may depend template deduction rules that have been updated in C++20). I'm not sure if the end result is better though. > The change itself is fine though > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > unsigned int i, w, h, cellSize; > > for (i = 0; i < numCells; i++) { > > cellSize = cellSizes[i]; > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > index 3579ba96b6ea..c1de1fee701a 100644 > > --- a/src/libcamera/camera.cpp > > +++ b/src/libcamera/camera.cpp > > @@ -7,6 +7,7 @@ > > > > #include <libcamera/camera.h> > > > > +#include <array> > > #include <atomic> > > #include <iomanip> > > > > @@ -17,7 +18,6 @@ > > #include "libcamera/internal/log.h" > > #include "libcamera/internal/pipeline_handler.h" > > #include "libcamera/internal/thread.h" > > -#include "libcamera/internal/utils.h" > > > > /** > > * \file camera.h > > @@ -393,7 +393,7 @@ int Camera::Private::isAccessAllowed(State state, bool allowDisconnected) const > > if (currentState == state) > > return 0; > > > > - ASSERT(static_cast<unsigned int>(state) < ARRAY_SIZE(camera_state_names)); > > + ASSERT(static_cast<unsigned int>(state) < std::size(camera_state_names)); > > > > LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] > > << " state trying operation requiring state " > > @@ -412,8 +412,8 @@ int Camera::Private::isAccessAllowed(State low, State high, > > if (currentState >= low && currentState <= high) > > return 0; > > > > - ASSERT(static_cast<unsigned int>(low) < ARRAY_SIZE(camera_state_names) && > > - static_cast<unsigned int>(high) < ARRAY_SIZE(camera_state_names)); > > + ASSERT(static_cast<unsigned int>(low) < std::size(camera_state_names) && > > + static_cast<unsigned int>(high) < std::size(camera_state_names)); > > > > LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] > > << " state trying operation requiring state between " > > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp > > index 180eb97ba664..45c7c2d24652 100644 > > --- a/src/libcamera/log.cpp > > +++ b/src/libcamera/log.cpp > > @@ -7,6 +7,7 @@ > > > > #include "libcamera/internal/log.h" > > > > +#include <array> > > #if HAVE_BACKTRACE > > #include <execinfo.h> > > #endif > > @@ -91,7 +92,7 @@ static const char *log_severity_name(LogSeverity severity) > > "FATAL", > > }; > > > > - if (static_cast<unsigned int>(severity) < ARRAY_SIZE(names)) > > + if (static_cast<unsigned int>(severity) < std::size(names)) > > return names[severity]; > > else > > return "UNKWN"; > > @@ -406,7 +407,7 @@ void Logger::backtrace() > > return; > > > > void *buffer[32]; > > - int num_entries = ::backtrace(buffer, ARRAY_SIZE(buffer)); > > + int num_entries = ::backtrace(buffer, std::size(buffer)); > > char **strings = backtrace_symbols(buffer, num_entries); > > if (!strings) > > return; > > @@ -620,7 +621,7 @@ LogSeverity Logger::parseLogLevel(const std::string &level) > > severity = LogInvalid; > > } else { > > severity = LogInvalid; > > - for (unsigned int i = 0; i < ARRAY_SIZE(names); ++i) { > > + for (unsigned int i = 0; i < std::size(names); ++i) { > > if (names[i] == level) { > > severity = i; > > break; > > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp > > index da85c9c24340..e90375ae115c 100644 > > --- a/src/libcamera/utils.cpp > > +++ b/src/libcamera/utils.cpp > > @@ -31,11 +31,6 @@ namespace libcamera { > > > > namespace utils { > > > > -/** > > - * \def ARRAY_SIZE(array) > > - * \brief Determine the number of elements in the static array. > > - */ > > - > > /** > > * \brief Strip the directory prefix from the path > > * \param[in] path The path to process > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > > index e2b582842a9b..baf683d6d985 100644 > > --- a/src/libcamera/v4l2_videodevice.cpp > > +++ b/src/libcamera/v4l2_videodevice.cpp > > @@ -7,6 +7,7 @@ > > > > #include "libcamera/internal/v4l2_videodevice.h" > > > > +#include <array> > > #include <fcntl.h> > > #include <iomanip> > > #include <sstream> > > @@ -26,7 +27,6 @@ > > #include "libcamera/internal/log.h" > > #include "libcamera/internal/media_device.h" > > #include "libcamera/internal/media_object.h" > > -#include "libcamera/internal/utils.h" > > > > /** > > * \file v4l2_videodevice.h > > @@ -860,7 +860,7 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set) > > pix->num_planes = format->planesCount; > > pix->field = V4L2_FIELD_NONE; > > > > - ASSERT(pix->num_planes <= ARRAY_SIZE(pix->plane_fmt)); > > + ASSERT(pix->num_planes <= std::size(pix->plane_fmt)); > > > > for (unsigned int i = 0; i < pix->num_planes; ++i) { > > pix->plane_fmt[i].bytesperline = format->planes[i].bpl; > > @@ -1255,7 +1255,7 @@ std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) > > buf.index = index; > > buf.type = bufferType_; > > buf.memory = V4L2_MEMORY_MMAP; > > - buf.length = ARRAY_SIZE(v4l2Planes); > > + buf.length = std::size(v4l2Planes); > > buf.m.planes = v4l2Planes; > > > > int ret = ioctl(VIDIOC_QUERYBUF, &buf); > > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp > > index 19a1d7dd8a2d..80157b342795 100644 > > --- a/test/ipc/unixsocket.cpp > > +++ b/test/ipc/unixsocket.cpp > > @@ -6,6 +6,7 @@ > > */ > > > > #include <algorithm> > > +#include <array> > > #include <fcntl.h> > > #include <iostream> > > #include <stdlib.h> > > @@ -19,7 +20,6 @@ > > #include "libcamera/internal/ipc_unixsocket.h" > > #include "libcamera/internal/thread.h" > > #include "libcamera/internal/timer.h" > > -#include "libcamera/internal/utils.h" > > > > #include "test.h" > > > > @@ -311,7 +311,7 @@ protected: > > }; > > int fds[2]; > > > > - for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { > > + for (unsigned int i = 0; i < std::size(strings); i++) { > > unsigned int len = strlen(strings[i]); > > > > fds[i] = open("/tmp", O_TMPFILE | O_RDWR, > > @@ -333,7 +333,7 @@ protected: > > if (ret) > > return ret; > > > > - for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { > > + for (unsigned int i = 0; i < std::size(strings); i++) { > > unsigned int len = strlen(strings[i]); > > char buf[len]; > >
diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h index ebb2c4038e19..f08134afb5ba 100644 --- a/include/libcamera/internal/utils.h +++ b/include/libcamera/internal/utils.h @@ -17,8 +17,6 @@ #include <sys/time.h> #include <vector> -#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0])) - #ifndef __DOXYGEN__ /* uClibc and uClibc-ng don't provide O_TMPFILE */ diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 801400214c3a..a7439badb7dc 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -6,6 +6,7 @@ */ #include <algorithm> +#include <array> #include <fcntl.h> #include <math.h> #include <stdint.h> @@ -27,7 +28,6 @@ #include "libcamera/internal/buffer.h" #include "libcamera/internal/camera_sensor.h" #include "libcamera/internal/log.h" -#include "libcamera/internal/utils.h" #include <linux/bcm2835-isp.h> @@ -1092,7 +1092,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls) * Choose smallest cell size that won't exceed 63x48 cells. */ const int cellSizes[] = { 16, 32, 64, 128, 256 }; - unsigned int numCells = ARRAY_SIZE(cellSizes); + unsigned int numCells = std::size(cellSizes); unsigned int i, w, h, cellSize; for (i = 0; i < numCells; i++) { cellSize = cellSizes[i]; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 3579ba96b6ea..c1de1fee701a 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -7,6 +7,7 @@ #include <libcamera/camera.h> +#include <array> #include <atomic> #include <iomanip> @@ -17,7 +18,6 @@ #include "libcamera/internal/log.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/thread.h" -#include "libcamera/internal/utils.h" /** * \file camera.h @@ -393,7 +393,7 @@ int Camera::Private::isAccessAllowed(State state, bool allowDisconnected) const if (currentState == state) return 0; - ASSERT(static_cast<unsigned int>(state) < ARRAY_SIZE(camera_state_names)); + ASSERT(static_cast<unsigned int>(state) < std::size(camera_state_names)); LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] << " state trying operation requiring state " @@ -412,8 +412,8 @@ int Camera::Private::isAccessAllowed(State low, State high, if (currentState >= low && currentState <= high) return 0; - ASSERT(static_cast<unsigned int>(low) < ARRAY_SIZE(camera_state_names) && - static_cast<unsigned int>(high) < ARRAY_SIZE(camera_state_names)); + ASSERT(static_cast<unsigned int>(low) < std::size(camera_state_names) && + static_cast<unsigned int>(high) < std::size(camera_state_names)); LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] << " state trying operation requiring state between " diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp index 180eb97ba664..45c7c2d24652 100644 --- a/src/libcamera/log.cpp +++ b/src/libcamera/log.cpp @@ -7,6 +7,7 @@ #include "libcamera/internal/log.h" +#include <array> #if HAVE_BACKTRACE #include <execinfo.h> #endif @@ -91,7 +92,7 @@ static const char *log_severity_name(LogSeverity severity) "FATAL", }; - if (static_cast<unsigned int>(severity) < ARRAY_SIZE(names)) + if (static_cast<unsigned int>(severity) < std::size(names)) return names[severity]; else return "UNKWN"; @@ -406,7 +407,7 @@ void Logger::backtrace() return; void *buffer[32]; - int num_entries = ::backtrace(buffer, ARRAY_SIZE(buffer)); + int num_entries = ::backtrace(buffer, std::size(buffer)); char **strings = backtrace_symbols(buffer, num_entries); if (!strings) return; @@ -620,7 +621,7 @@ LogSeverity Logger::parseLogLevel(const std::string &level) severity = LogInvalid; } else { severity = LogInvalid; - for (unsigned int i = 0; i < ARRAY_SIZE(names); ++i) { + for (unsigned int i = 0; i < std::size(names); ++i) { if (names[i] == level) { severity = i; break; diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp index da85c9c24340..e90375ae115c 100644 --- a/src/libcamera/utils.cpp +++ b/src/libcamera/utils.cpp @@ -31,11 +31,6 @@ namespace libcamera { namespace utils { -/** - * \def ARRAY_SIZE(array) - * \brief Determine the number of elements in the static array. - */ - /** * \brief Strip the directory prefix from the path * \param[in] path The path to process diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index e2b582842a9b..baf683d6d985 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -7,6 +7,7 @@ #include "libcamera/internal/v4l2_videodevice.h" +#include <array> #include <fcntl.h> #include <iomanip> #include <sstream> @@ -26,7 +27,6 @@ #include "libcamera/internal/log.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/media_object.h" -#include "libcamera/internal/utils.h" /** * \file v4l2_videodevice.h @@ -860,7 +860,7 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set) pix->num_planes = format->planesCount; pix->field = V4L2_FIELD_NONE; - ASSERT(pix->num_planes <= ARRAY_SIZE(pix->plane_fmt)); + ASSERT(pix->num_planes <= std::size(pix->plane_fmt)); for (unsigned int i = 0; i < pix->num_planes; ++i) { pix->plane_fmt[i].bytesperline = format->planes[i].bpl; @@ -1255,7 +1255,7 @@ std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) buf.index = index; buf.type = bufferType_; buf.memory = V4L2_MEMORY_MMAP; - buf.length = ARRAY_SIZE(v4l2Planes); + buf.length = std::size(v4l2Planes); buf.m.planes = v4l2Planes; int ret = ioctl(VIDIOC_QUERYBUF, &buf); diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp index 19a1d7dd8a2d..80157b342795 100644 --- a/test/ipc/unixsocket.cpp +++ b/test/ipc/unixsocket.cpp @@ -6,6 +6,7 @@ */ #include <algorithm> +#include <array> #include <fcntl.h> #include <iostream> #include <stdlib.h> @@ -19,7 +20,6 @@ #include "libcamera/internal/ipc_unixsocket.h" #include "libcamera/internal/thread.h" #include "libcamera/internal/timer.h" -#include "libcamera/internal/utils.h" #include "test.h" @@ -311,7 +311,7 @@ protected: }; int fds[2]; - for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { + for (unsigned int i = 0; i < std::size(strings); i++) { unsigned int len = strlen(strings[i]); fds[i] = open("/tmp", O_TMPFILE | O_RDWR, @@ -333,7 +333,7 @@ protected: if (ret) return ret; - for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { + for (unsigned int i = 0; i < std::size(strings); i++) { unsigned int len = strlen(strings[i]); char buf[len];
C++17 has a std::size() function that returns the size of a C-style array. Use it instead of the custom ARRAY_SIZE macro. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/internal/utils.h | 2 -- src/ipa/raspberrypi/raspberrypi.cpp | 4 ++-- src/libcamera/camera.cpp | 8 ++++---- src/libcamera/log.cpp | 7 ++++--- src/libcamera/utils.cpp | 5 ----- src/libcamera/v4l2_videodevice.cpp | 6 +++--- test/ipc/unixsocket.cpp | 6 +++--- 7 files changed, 16 insertions(+), 22 deletions(-)