[libcamera-devel] [PATCH 01/17] meson: Allow partially initializing objects
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Jun 9 12:18:50 CEST 2019
Hi Kieran,
On Sun, Jun 09, 2019 at 01:26:00AM +0100, Kieran Bingham wrote:
> On 08/06/2019 17:20, Laurent Pinchart wrote:
> > On Fri, May 31, 2019 at 11:10:10AM +0100, Kieran Bingham wrote:
> >> On 29/05/2019 22:13, Kieran Bingham wrote:
> >>> On 27/05/2019 10:18, Jacopo Mondi wrote:
> >>>> On Mon, May 27, 2019 at 02:15:27AM +0200, Niklas Söderlund wrote:
> >>>>> There are valid use-cases where one might wish to partially initialize a
> >>>>> structure when creating it without needing to initialize all members.
> >>>>> This is especially common when working with V4L2.
> >>>>>
> >>>>> struct foo {
> >>>>> int bar;
> >>>>> int baz;
> >>>>> };
> >>>>>
> >>>>> struct foo f = {
> >>>>> .bar = 1;
> >>>>> };
> >>>>>
> >>>>> Without -Wno-missing-field-initializers fails to compile with the error,
> >>>>
> >>>> I asked for the same a few weeks ago, so I welcome this change
> >>>> Acked-by: Jacopo Mondi <jacopo at jmondi.org>
> >>>
> >>> I was uncertain before, but I can't deny it would be useful to simplify
> >>> code /iff/ you know certain members do not need to be initialised.
> >>>
> >>> We'll have to watch our reviews more closely on initialising structures :-)
> >>>
> >>> Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >>
> >> Errr,,, uhmmm ... sorry - Ack retracted? :-(
> >>
> >> This breaks compilation on GCC v5 and GCC v6.
> >>
> >> (Raspbian has GCC v6.3, and compilation on the target fails.)
> >
> > Could you please share the error messages ?
>
> Ah yes, sorry - that might have been useful content:
>
> c++ -Isrc/libcamera/4ab8042@@camera at sha -Isrc/libcamera
> -I../src/libcamera -Iinclude -I../include -I../src/libcamera/include
> -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall
> -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++11 -g
> -Wno-unused-parameter -Wno-missing-field-initializers -include config.h
> -fPIC -MD -MQ 'src/libcamera/4ab8042@@camera at sha/v4l2_subdevice.cpp.o'
> -MF 'src/libcamera/4ab8042@@camera at sha/v4l2_subdevice.cpp.o.d' -o
> 'src/libcamera/4ab8042@@camera at sha/v4l2_subdevice.cpp.o' -c
> ../src/libcamera/v4l2_subdevice.cpp
> ../src/libcamera/v4l2_subdevice.cpp: In member function
> ‘std::vector<unsigned int>
> libcamera::V4L2Subdevice::enumPadCodes(unsigned int)’:
> ../src/libcamera/v4l2_subdevice.cpp:517:3: sorry, unimplemented:
> non-trivial designated initializers not supported
> };
> ^
> ../src/libcamera/v4l2_subdevice.cpp: In member function
> ‘std::vector<libcamera::SizeRange>
> libcamera::V4L2Subdevice::enumPadSizes(unsigned int, unsigned int)’:
> ../src/libcamera/v4l2_subdevice.cpp:549:3: sorry, unimplemented:
> non-trivial designated initializers not supported
> };
> ^
>
> "Sorry, unimplemented:" - well at least they apologised :-D
>
> I'm not sure what we could do here. It sounds like they simply hadn't
> implemented the 'feature' at this point.
>
> We could explicitly state that our minimum compiler requirement is
> GCCv7+ (once we verify GCCv7 supports our requirements)?
The issue is that aggregate designated initializers have been introduced
in C++20, so we can't depend on it. I see two options, either upgrading
our minimum compiler version and hoping that it will just work (without
any formal guarantee that newer compiler versions will handle it
correctly), or stop using designated initializers. If we go for the
second option, this patch won't be needed.
> (gcc version information from the target below:)
>
> gcc -v
> Using built-in specs.
> COLLECT_GCC=gcc
> COLLECT_LTO_WRAPPER=/usr/lib/gcc/arm-linux-gnueabihf/6/lto-wrapper
> Target: arm-linux-gnueabihf
> Configured with: ../src/configure -v --with-pkgversion='Raspbian
> 6.3.0-18+rpi1+deb9u1'
> --with-bugurl=file:///usr/share/doc/gcc-6/README.Bugs
> --enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-c++
> --prefix=/usr --program-suffix=-6 --program-prefix=arm-linux-gnueabihf-
> --enable-shared --enable-linker-build-id --libexecdir=/usr/lib
> --without-included-gettext --enable-threads=posix --libdir=/usr/lib
> --enable-nls --with-sysroot=/ --enable-clocale=gnu
> --enable-libstdcxx-debug --enable-libstdcxx-time=yes
> --with-default-libstdcxx-abi=new --enable-gnu-unique-object
> --disable-libitm --disable-libquadmath --enable-plugin
> --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk
> --enable-gtk-cairo
> --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-6-armhf/jre
> --enable-java-home
> --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-6-armhf
> --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-6-armhf
> --with-arch-directory=arm --with-ecj-jar=/usr/share/java/eclipse-ecj.jar
> --with-target-system-zlib --enable-objc-gc=auto --enable-multiarch
> --disable-sjlj-exceptions --with-arch=armv6 --with-fpu=vfp
> --with-float=hard --enable-checking=release --build=arm-linux-gnueabihf
> --host=arm-linux-gnueabihf --target=arm-linux-gnueabihf
> Thread model: posix
> gcc version 6.3.0 20170516 (Raspbian 6.3.0-18+rpi1+deb9u1)
>
> >> To fix, I had to make the following change for this series.
> >> Other solutions may also exist :-)
> >>
> >>> @@ -309,15 +309,15 @@ std::vector<unsigned int> V4L2Subdevice::enumPadCodes(unsigned int pad)
> >>> {
> >>> std::vector<unsigned int> codes;
> >>> int ret;
> >>>
> >>> for (unsigned int index = 0;; index++) {
> >>> - struct v4l2_subdev_mbus_code_enum mbusEnum = {
> >>> - .pad = pad,
> >>> - .index = index,
> >>> - .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> >>> - };
> >>> + struct v4l2_subdev_mbus_code_enum mbusEnum;
> >>> +
> >>> + mbusEnum.pad = pad;
> >>> + mbusEnum.index = index;
> >>> + mbusEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> >>>
> >>> ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_MBUS_CODE, &mbusEnum);
> >>> if (ret)
> >>> break;
> >>>
> >>> @@ -340,16 +340,16 @@ std::vector<SizeRange> V4L2Subdevice::enumPadSizes(unsigned int pad,
> >>> {
> >>> std::vector<SizeRange> sizes;
> >>> int ret;
> >>>
> >>> for (unsigned int index = 0;; index++) {
> >>> - struct v4l2_subdev_frame_size_enum sizeEnum = {
> >>> - .index = index,
> >>> - .pad = pad,
> >>> - .code = code,
> >>> - .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> >>> - };
> >>> + struct v4l2_subdev_frame_size_enum sizeEnum;
> >>> +
> >>> + sizeEnum.index = index;
> >>> + sizeEnum.pad = pad;
> >>> + sizeEnum.code = code;
> >>> + sizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> >>>
> >>> ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &sizeEnum);
> >>> if (ret)
> >>> break;
> >>>
> >>>>> error: missing initializer for member ‘foo::baz’ [-Werror=missing-field-initializers]
> >>>>>
> >>>>> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >>>>> ---
> >>>>> meson.build | 1 +
> >>>>> 1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/meson.build b/meson.build
> >>>>> index 4d3e99d3e58f71d5..90ad58328f9a2bd5 100644
> >>>>> --- a/meson.build
> >>>>> +++ b/meson.build
> >>>>> @@ -22,6 +22,7 @@ endif
> >>>>>
> >>>>> common_arguments = [
> >>>>> '-Wno-unused-parameter',
> >>>>> + '-Wno-missing-field-initializers',
> >>>>> '-include', 'config.h',
> >>>>> ]
> >>>>>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list