[libcamera-devel,4/4] libcamera: Standardise on C compatibility headers

Message ID 20191023135258.32256-4-laurent.pinchart@ideasonboard.com
State Accepted
Commit ca260d2f5337536e30d1b2501434bd2a93a3e72e
Headers show
Series
  • [libcamera-devel,1/4] Documentation: coding-style: Document usage of C compatibility headers
Related show

Commit Message

Laurent Pinchart Oct. 23, 2019, 1:52 p.m. UTC
Now that our usage of C compatibility header is documented, use them
consistently through the source code. While at it, group the C and C++
include statements, and fix a handful of #include ordering issues.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/timer.h                     | 2 +-
 src/cam/capture.cpp                           | 2 +-
 src/cam/options.cpp                           | 2 +-
 src/ipa/rkisp1/rkisp1.cpp                     | 2 +-
 src/libcamera/device_enumerator_udev.cpp      | 5 ++---
 src/libcamera/include/event_dispatcher_poll.h | 4 ++--
 src/libcamera/include/ipc_unixsocket.h        | 2 +-
 src/libcamera/log.cpp                         | 6 +++---
 src/libcamera/media_device.cpp                | 3 +--
 src/libcamera/media_object.cpp                | 3 +--
 src/libcamera/stream.cpp                      | 2 +-
 test/ipa/ipa_interface_test.cpp               | 3 +--
 test/media_device/media_device_print_test.cpp | 2 +-
 test/pipeline/ipu3/ipu3_pipeline_test.cpp     | 1 -
 test/v4l2_subdevice/test_formats.cpp          | 2 +-
 test/v4l2_videodevice/capture_async.cpp       | 4 ++--
 test/v4l2_videodevice/controls.cpp            | 2 +-
 test/v4l2_videodevice/formats.cpp             | 2 +-
 18 files changed, 22 insertions(+), 27 deletions(-)

Comments

Kieran Bingham Oct. 23, 2019, 2:24 p.m. UTC | #1
Hi Laurent,

On 23/10/2019 14:52, Laurent Pinchart wrote:
> Now that our usage of C compatibility header is documented, use them
> consistently through the source code. While at it, group the C and C++
> include statements, and fix a handful of #include ordering issues.

I think the 'handful' of include ordering issues is mostly where we
previously separated C and C++ header includes.

If we're removing this separation explicitly - I think stating that
clearly here would be good.

discussion comment down at the event_dispatcher_poll too...

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

But otherwise, with those comments resolved in whichever way you deem best,

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>




> ---
>  include/libcamera/timer.h                     | 2 +-
>  src/cam/capture.cpp                           | 2 +-
>  src/cam/options.cpp                           | 2 +-
>  src/ipa/rkisp1/rkisp1.cpp                     | 2 +-
>  src/libcamera/device_enumerator_udev.cpp      | 5 ++---
>  src/libcamera/include/event_dispatcher_poll.h | 4 ++--
>  src/libcamera/include/ipc_unixsocket.h        | 2 +-
>  src/libcamera/log.cpp                         | 6 +++---
>  src/libcamera/media_device.cpp                | 3 +--
>  src/libcamera/media_object.cpp                | 3 +--
>  src/libcamera/stream.cpp                      | 2 +-
>  test/ipa/ipa_interface_test.cpp               | 3 +--
>  test/media_device/media_device_print_test.cpp | 2 +-
>  test/pipeline/ipu3/ipu3_pipeline_test.cpp     | 1 -
>  test/v4l2_subdevice/test_formats.cpp          | 2 +-
>  test/v4l2_videodevice/capture_async.cpp       | 4 ++--
>  test/v4l2_videodevice/controls.cpp            | 2 +-
>  test/v4l2_videodevice/formats.cpp             | 2 +-
>  18 files changed, 22 insertions(+), 27 deletions(-)
> 
> diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h
> index 34e7b8ac8e87..f55fe3c09660 100644
> --- a/include/libcamera/timer.h
> +++ b/include/libcamera/timer.h
> @@ -8,7 +8,7 @@
>  #define __LIBCAMERA_TIMER_H__
>  
>  #include <chrono>
> -#include <cstdint>
> +#include <stdint.h>
>  
>  #include <libcamera/object.h>
>  #include <libcamera/signal.h>
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index 8a939c622703..d05c01f1b13e 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -6,9 +6,9 @@
>   */
>  
>  #include <chrono>
> -#include <climits>
>  #include <iomanip>
>  #include <iostream>
> +#include <limits.h>
>  #include <sstream>
>  
>  #include "capture.h"
> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> index 7c3948df3b5c..1f0631eccfba 100644
> --- a/src/cam/options.cpp
> +++ b/src/cam/options.cpp
> @@ -5,7 +5,7 @@
>   * options.cpp - cam - Options parsing
>   */
>  
> -#include <cassert>
> +#include <assert.h>
>  #include <getopt.h>
>  #include <iomanip>
>  #include <iostream>
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 570145ce71b2..9a13f5c7df17 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -6,9 +6,9 @@
>   */
>  
>  #include <algorithm>
> -#include <cstdint>
>  #include <math.h>
>  #include <queue>
> +#include <stdint.h>
>  #include <string.h>
>  
>  #include <linux/rkisp1-config.h>
> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> index ddcd59ea52c1..b2c5fd221dcd 100644
> --- a/src/libcamera/device_enumerator_udev.cpp
> +++ b/src/libcamera/device_enumerator_udev.cpp
> @@ -8,11 +8,10 @@
>  #include "device_enumerator_udev.h"
>  
>  #include <algorithm>
> -#include <list>
> -#include <map>
> -
>  #include <fcntl.h>
>  #include <libudev.h>
> +#include <list>
> +#include <map>
>  #include <string.h>
>  #include <sys/ioctl.h>
>  #include <sys/sysmacros.h>
> diff --git a/src/libcamera/include/event_dispatcher_poll.h b/src/libcamera/include/event_dispatcher_poll.h
> index d82b302c4aea..1f0738617425 100644
> --- a/src/libcamera/include/event_dispatcher_poll.h
> +++ b/src/libcamera/include/event_dispatcher_poll.h
> @@ -7,12 +7,12 @@
>  #ifndef __LIBCAMERA_EVENT_DISPATCHER_POLL_H__
>  #define __LIBCAMERA_EVENT_DISPATCHER_POLL_H__
>  
> -#include <libcamera/event_dispatcher.h>
> -
>  #include <list>
>  #include <map>
>  #include <vector>
>  
> +#include <libcamera/event_dispatcher.h>

>From 2/4

+The headers shall be grouped and ordered as follows.
+
+ # The header declaring the API being implemented (if any)
+ # The C and C++ system and standard library headers
+ # Other libraries' headers, with one group per library
+ # Other project's headers

Now that's a possible confusion point, /this/ EVENT_DISPATCHER_POLL
header /is/ implementing the API for an event dispatcher - so therefore
point one above could be seen to be declaring it should be first.

However, I suspect the intent of the ordering might be that it should
only be first if this is the header for this exact implementation, not
any virtual interface. But I don't know how to word that to remove
ambiguity :D

Or maybe this one should be first ... I'll leave it to you ...

> +
>  struct pollfd;
>  
>  namespace libcamera {
> diff --git a/src/libcamera/include/ipc_unixsocket.h b/src/libcamera/include/ipc_unixsocket.h
> index 03e9fe492bde..820d05611049 100644
> --- a/src/libcamera/include/ipc_unixsocket.h
> +++ b/src/libcamera/include/ipc_unixsocket.h
> @@ -8,7 +8,7 @@
>  #ifndef __LIBCAMERA_IPC_UNIXSOCKET_H__
>  #define __LIBCAMERA_IPC_UNIXSOCKET_H__
>  
> -#include <cstdint>
> +#include <stdint.h>
>  #include <sys/types.h>
>  #include <vector>
>  
> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp
> index 51f9f86b4c44..50f345b98c74 100644
> --- a/src/libcamera/log.cpp
> +++ b/src/libcamera/log.cpp
> @@ -7,14 +7,14 @@
>  
>  #include "log.h"
>  
> -#include <cstdio>
> -#include <cstdlib>
> -#include <ctime>
>  #include <fstream>
>  #include <iostream>
>  #include <list>
> +#include <stdio.h>
> +#include <stdlib.h>
>  #include <string.h>
>  #include <syslog.h>
> +#include <time.h>
>  #include <unordered_set>
>  
>  #include <libcamera/logging.h>
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index bd99deba098d..0d6630fbd1f4 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -9,11 +9,10 @@
>  
>  #include <errno.h>
>  #include <fcntl.h>
> +#include <string>
>  #include <string.h>
>  #include <sys/ioctl.h>
>  #include <unistd.h>
> -
> -#include <string>
>  #include <vector>

I kind of really like the separation between C headers and C++
headers... but ho hum. It's arbitrary.


>  #include <linux/media.h>
> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
> index 8794ff4578c9..ef32065c161d 100644
> --- a/src/libcamera/media_object.cpp
> +++ b/src/libcamera/media_object.cpp
> @@ -8,10 +8,9 @@
>  #include "media_object.h"
>  
>  #include <errno.h>
> +#include <string>
>  #include <string.h>
>  #include <unistd.h>
> -
> -#include <string>
>  #include <vector>
>  
>  #include <linux/media.h>
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index 610920d1e5b3..b8e7209c1047 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -9,8 +9,8 @@
>  
>  #include <algorithm>
>  #include <array>
> -#include <climits>
>  #include <iomanip>
> +#include <limits.h>
>  #include <sstream>
>  
>  #include <libcamera/request.h>
> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
> index 0bdeb167a675..cafc249bbbc9 100644
> --- a/test/ipa/ipa_interface_test.cpp
> +++ b/test/ipa/ipa_interface_test.cpp
> @@ -6,13 +6,12 @@
>   */
>  
>  #include <fcntl.h>
> +#include <iostream>
>  #include <string.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  #include <unistd.h>
>  
> -#include <iostream>
> -

Hrm ... this does show that we have considered C and C++ headers as
separate groups previously - so this is definitely a change ... Worth
checking specifically with the others.

Either way, consistency is good.


>  #include <libcamera/event_dispatcher.h>
>  #include <libcamera/event_notifier.h>
>  #include <libcamera/timer.h>
> diff --git a/test/media_device/media_device_print_test.cpp b/test/media_device/media_device_print_test.cpp
> index 30d929b8c763..8dd8a151266d 100644
> --- a/test/media_device/media_device_print_test.cpp
> +++ b/test/media_device/media_device_print_test.cpp
> @@ -4,8 +4,8 @@
>   *
>   * media_device_print_test.cpp - Print out media devices
>   */
> -#include <iostream>
>  
> +#include <iostream>
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <unistd.h>
> diff --git a/test/pipeline/ipu3/ipu3_pipeline_test.cpp b/test/pipeline/ipu3/ipu3_pipeline_test.cpp
> index 8bfcd609a071..a5c6be0955df 100644
> --- a/test/pipeline/ipu3/ipu3_pipeline_test.cpp
> +++ b/test/pipeline/ipu3/ipu3_pipeline_test.cpp
> @@ -6,7 +6,6 @@
>   */
>  
>  #include <iostream>
> -
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  #include <unistd.h>
> diff --git a/test/v4l2_subdevice/test_formats.cpp b/test/v4l2_subdevice/test_formats.cpp
> index e90c2c2426da..5cf5d5664b04 100644
> --- a/test/v4l2_subdevice/test_formats.cpp
> +++ b/test/v4l2_subdevice/test_formats.cpp
> @@ -5,8 +5,8 @@
>   * libcamera V4L2 Subdevice format handling test
>   */
>  
> -#include <climits>
>  #include <iostream>
> +#include <limits.h>
>  
>  #include "v4l2_subdevice.h"
>  #include "v4l2_subdevice_test.h"
> diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp
> index 17eb528b12fd..442a4fe56eac 100644
> --- a/test/v4l2_videodevice/capture_async.cpp
> +++ b/test/v4l2_videodevice/capture_async.cpp
> @@ -5,12 +5,12 @@
>   * libcamera V4L2 API tests
>   */
>  
> +#include <iostream>
> +
>  #include <libcamera/buffer.h>
>  #include <libcamera/event_dispatcher.h>
>  #include <libcamera/timer.h>
>  
> -#include <iostream>
> -
>  #include "thread.h"
>  #include "v4l2_videodevice_test.h"
>  
> diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
> index 975c852b8cbf..42c653d4435a 100644
> --- a/test/v4l2_videodevice/controls.cpp
> +++ b/test/v4l2_videodevice/controls.cpp
> @@ -5,8 +5,8 @@
>   * controls.cpp - V4L2 device controls handling test
>   */
>  
> -#include <climits>
>  #include <iostream>
> +#include <limits.h>
>  
>  #include "v4l2_videodevice.h"
>  
> diff --git a/test/v4l2_videodevice/formats.cpp b/test/v4l2_videodevice/formats.cpp
> index ee7d357de075..d504d1788d02 100644
> --- a/test/v4l2_videodevice/formats.cpp
> +++ b/test/v4l2_videodevice/formats.cpp
> @@ -5,8 +5,8 @@
>   * libcamera V4L2 device format handling test
>   */
>  
> -#include <climits>
>  #include <iostream>
> +#include <limits.h>
>  
>  #include "v4l2_videodevice.h"
>  
>
Laurent Pinchart Oct. 23, 2019, 2:39 p.m. UTC | #2
On Wed, Oct 23, 2019 at 03:24:33PM +0100, Kieran Bingham wrote:
> Hi Laurent,
> 
> On 23/10/2019 14:52, Laurent Pinchart wrote:
> > Now that our usage of C compatibility header is documented, use them
> > consistently through the source code. While at it, group the C and C++
> > include statements, and fix a handful of #include ordering issues.
> 
> I think the 'handful' of include ordering issues is mostly where we
> previously separated C and C++ header includes.
> 
> If we're removing this separation explicitly - I think stating that
> clearly here would be good.

I'll expand the commit message.

> discussion comment down at the event_dispatcher_poll too...
> 
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> But otherwise, with those comments resolved in whichever way you deem best,
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > ---
> >  include/libcamera/timer.h                     | 2 +-
> >  src/cam/capture.cpp                           | 2 +-
> >  src/cam/options.cpp                           | 2 +-
> >  src/ipa/rkisp1/rkisp1.cpp                     | 2 +-
> >  src/libcamera/device_enumerator_udev.cpp      | 5 ++---
> >  src/libcamera/include/event_dispatcher_poll.h | 4 ++--
> >  src/libcamera/include/ipc_unixsocket.h        | 2 +-
> >  src/libcamera/log.cpp                         | 6 +++---
> >  src/libcamera/media_device.cpp                | 3 +--
> >  src/libcamera/media_object.cpp                | 3 +--
> >  src/libcamera/stream.cpp                      | 2 +-
> >  test/ipa/ipa_interface_test.cpp               | 3 +--
> >  test/media_device/media_device_print_test.cpp | 2 +-
> >  test/pipeline/ipu3/ipu3_pipeline_test.cpp     | 1 -
> >  test/v4l2_subdevice/test_formats.cpp          | 2 +-
> >  test/v4l2_videodevice/capture_async.cpp       | 4 ++--
> >  test/v4l2_videodevice/controls.cpp            | 2 +-
> >  test/v4l2_videodevice/formats.cpp             | 2 +-
> >  18 files changed, 22 insertions(+), 27 deletions(-)
> > 
> > diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h
> > index 34e7b8ac8e87..f55fe3c09660 100644
> > --- a/include/libcamera/timer.h
> > +++ b/include/libcamera/timer.h
> > @@ -8,7 +8,7 @@
> >  #define __LIBCAMERA_TIMER_H__
> >  
> >  #include <chrono>
> > -#include <cstdint>
> > +#include <stdint.h>
> >  
> >  #include <libcamera/object.h>
> >  #include <libcamera/signal.h>
> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> > index 8a939c622703..d05c01f1b13e 100644
> > --- a/src/cam/capture.cpp
> > +++ b/src/cam/capture.cpp
> > @@ -6,9 +6,9 @@
> >   */
> >  
> >  #include <chrono>
> > -#include <climits>
> >  #include <iomanip>
> >  #include <iostream>
> > +#include <limits.h>
> >  #include <sstream>
> >  
> >  #include "capture.h"
> > diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> > index 7c3948df3b5c..1f0631eccfba 100644
> > --- a/src/cam/options.cpp
> > +++ b/src/cam/options.cpp
> > @@ -5,7 +5,7 @@
> >   * options.cpp - cam - Options parsing
> >   */
> >  
> > -#include <cassert>
> > +#include <assert.h>
> >  #include <getopt.h>
> >  #include <iomanip>
> >  #include <iostream>
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index 570145ce71b2..9a13f5c7df17 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -6,9 +6,9 @@
> >   */
> >  
> >  #include <algorithm>
> > -#include <cstdint>
> >  #include <math.h>
> >  #include <queue>
> > +#include <stdint.h>
> >  #include <string.h>
> >  
> >  #include <linux/rkisp1-config.h>
> > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> > index ddcd59ea52c1..b2c5fd221dcd 100644
> > --- a/src/libcamera/device_enumerator_udev.cpp
> > +++ b/src/libcamera/device_enumerator_udev.cpp
> > @@ -8,11 +8,10 @@
> >  #include "device_enumerator_udev.h"
> >  
> >  #include <algorithm>
> > -#include <list>
> > -#include <map>
> > -
> >  #include <fcntl.h>
> >  #include <libudev.h>
> > +#include <list>
> > +#include <map>
> >  #include <string.h>
> >  #include <sys/ioctl.h>
> >  #include <sys/sysmacros.h>
> > diff --git a/src/libcamera/include/event_dispatcher_poll.h b/src/libcamera/include/event_dispatcher_poll.h
> > index d82b302c4aea..1f0738617425 100644
> > --- a/src/libcamera/include/event_dispatcher_poll.h
> > +++ b/src/libcamera/include/event_dispatcher_poll.h
> > @@ -7,12 +7,12 @@
> >  #ifndef __LIBCAMERA_EVENT_DISPATCHER_POLL_H__
> >  #define __LIBCAMERA_EVENT_DISPATCHER_POLL_H__
> >  
> > -#include <libcamera/event_dispatcher.h>
> > -
> >  #include <list>
> >  #include <map>
> >  #include <vector>
> >  
> > +#include <libcamera/event_dispatcher.h>
> 
> From 2/4
> 
> +The headers shall be grouped and ordered as follows.
> +
> + # The header declaring the API being implemented (if any)
> + # The C and C++ system and standard library headers
> + # Other libraries' headers, with one group per library
> + # Other project's headers
> 
> Now that's a possible confusion point, /this/ EVENT_DISPATCHER_POLL
> header /is/ implementing the API for an event dispatcher - so therefore
> point one above could be seen to be declaring it should be first.

Nitpicking mode :-)

First of all, this is a header file, and thus doesn't implement the API,
it declares it (we could argue that headers can include inline
functions, but I don't think that's really relevant). Furthermore, the
documentation also states

"For .cpp files, if the file implements an API declared in a header file,
that header file shall be included first in order to ensure it is
self-contained."

so I would say that this rule is documented clearly enough as to only
apply to .cpp files.

Then, even if this was event_dispatcher_poll.cpp, I would argue that the
file implements the API declared in event_dispatcher_poll.h, not the one
defiend in event_dispatcher.h. We can say that it implements a class
that implements the interface (using it in the java sense of the term)
declared in event_dispatcher.h, but it doesn't really implement the API
declared there.

"the API being implemented" could be replaced by something clearer, but
I'll then leave it to you to propose a better wording in a patch :-)

> However, I suspect the intent of the ordering might be that it should
> only be first if this is the header for this exact implementation, not
> any virtual interface. But I don't know how to word that to remove
> ambiguity :D
> 
> Or maybe this one should be first ... I'll leave it to you ...

We already ensure that the header is self-contained in
event_dispatcher.cpp, which I think is the right place, so there's no
need to do it here (even if doinf so wouldn't hurt).

> > +
> >  struct pollfd;
> >  
> >  namespace libcamera {
> > diff --git a/src/libcamera/include/ipc_unixsocket.h b/src/libcamera/include/ipc_unixsocket.h
> > index 03e9fe492bde..820d05611049 100644
> > --- a/src/libcamera/include/ipc_unixsocket.h
> > +++ b/src/libcamera/include/ipc_unixsocket.h
> > @@ -8,7 +8,7 @@
> >  #ifndef __LIBCAMERA_IPC_UNIXSOCKET_H__
> >  #define __LIBCAMERA_IPC_UNIXSOCKET_H__
> >  
> > -#include <cstdint>
> > +#include <stdint.h>
> >  #include <sys/types.h>
> >  #include <vector>
> >  
> > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp
> > index 51f9f86b4c44..50f345b98c74 100644
> > --- a/src/libcamera/log.cpp
> > +++ b/src/libcamera/log.cpp
> > @@ -7,14 +7,14 @@
> >  
> >  #include "log.h"
> >  
> > -#include <cstdio>
> > -#include <cstdlib>
> > -#include <ctime>
> >  #include <fstream>
> >  #include <iostream>
> >  #include <list>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> >  #include <string.h>
> >  #include <syslog.h>
> > +#include <time.h>
> >  #include <unordered_set>
> >  
> >  #include <libcamera/logging.h>
> > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> > index bd99deba098d..0d6630fbd1f4 100644
> > --- a/src/libcamera/media_device.cpp
> > +++ b/src/libcamera/media_device.cpp
> > @@ -9,11 +9,10 @@
> >  
> >  #include <errno.h>
> >  #include <fcntl.h>
> > +#include <string>
> >  #include <string.h>
> >  #include <sys/ioctl.h>
> >  #include <unistd.h>
> > -
> > -#include <string>
> >  #include <vector>
> 
> I kind of really like the separation between C headers and C++
> headers... but ho hum. It's arbitrary.

We could split them, my only requirement is consistency :-) I prefer
grouping them (but that's only a mild personal preference) because we
would then split the C++-defined C compatibility headers from the other
C++ standard library headers.

> >  #include <linux/media.h>
> > diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
> > index 8794ff4578c9..ef32065c161d 100644
> > --- a/src/libcamera/media_object.cpp
> > +++ b/src/libcamera/media_object.cpp
> > @@ -8,10 +8,9 @@
> >  #include "media_object.h"
> >  
> >  #include <errno.h>
> > +#include <string>
> >  #include <string.h>
> >  #include <unistd.h>
> > -
> > -#include <string>
> >  #include <vector>
> >  
> >  #include <linux/media.h>
> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> > index 610920d1e5b3..b8e7209c1047 100644
> > --- a/src/libcamera/stream.cpp
> > +++ b/src/libcamera/stream.cpp
> > @@ -9,8 +9,8 @@
> >  
> >  #include <algorithm>
> >  #include <array>
> > -#include <climits>
> >  #include <iomanip>
> > +#include <limits.h>
> >  #include <sstream>
> >  
> >  #include <libcamera/request.h>
> > diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
> > index 0bdeb167a675..cafc249bbbc9 100644
> > --- a/test/ipa/ipa_interface_test.cpp
> > +++ b/test/ipa/ipa_interface_test.cpp
> > @@ -6,13 +6,12 @@
> >   */
> >  
> >  #include <fcntl.h>
> > +#include <iostream>
> >  #include <string.h>
> >  #include <sys/stat.h>
> >  #include <sys/types.h>
> >  #include <unistd.h>
> >  
> > -#include <iostream>
> > -
> 
> Hrm ... this does show that we have considered C and C++ headers as
> separate groups previously - so this is definitely a change ... Worth
> checking specifically with the others.
> 
> Either way, consistency is good.
> 
> >  #include <libcamera/event_dispatcher.h>
> >  #include <libcamera/event_notifier.h>
> >  #include <libcamera/timer.h>
> > diff --git a/test/media_device/media_device_print_test.cpp b/test/media_device/media_device_print_test.cpp
> > index 30d929b8c763..8dd8a151266d 100644
> > --- a/test/media_device/media_device_print_test.cpp
> > +++ b/test/media_device/media_device_print_test.cpp
> > @@ -4,8 +4,8 @@
> >   *
> >   * media_device_print_test.cpp - Print out media devices
> >   */
> > -#include <iostream>
> >  
> > +#include <iostream>
> >  #include <sys/types.h>
> >  #include <sys/stat.h>
> >  #include <unistd.h>
> > diff --git a/test/pipeline/ipu3/ipu3_pipeline_test.cpp b/test/pipeline/ipu3/ipu3_pipeline_test.cpp
> > index 8bfcd609a071..a5c6be0955df 100644
> > --- a/test/pipeline/ipu3/ipu3_pipeline_test.cpp
> > +++ b/test/pipeline/ipu3/ipu3_pipeline_test.cpp
> > @@ -6,7 +6,6 @@
> >   */
> >  
> >  #include <iostream>
> > -
> >  #include <sys/stat.h>
> >  #include <sys/types.h>
> >  #include <unistd.h>
> > diff --git a/test/v4l2_subdevice/test_formats.cpp b/test/v4l2_subdevice/test_formats.cpp
> > index e90c2c2426da..5cf5d5664b04 100644
> > --- a/test/v4l2_subdevice/test_formats.cpp
> > +++ b/test/v4l2_subdevice/test_formats.cpp
> > @@ -5,8 +5,8 @@
> >   * libcamera V4L2 Subdevice format handling test
> >   */
> >  
> > -#include <climits>
> >  #include <iostream>
> > +#include <limits.h>
> >  
> >  #include "v4l2_subdevice.h"
> >  #include "v4l2_subdevice_test.h"
> > diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp
> > index 17eb528b12fd..442a4fe56eac 100644
> > --- a/test/v4l2_videodevice/capture_async.cpp
> > +++ b/test/v4l2_videodevice/capture_async.cpp
> > @@ -5,12 +5,12 @@
> >   * libcamera V4L2 API tests
> >   */
> >  
> > +#include <iostream>
> > +
> >  #include <libcamera/buffer.h>
> >  #include <libcamera/event_dispatcher.h>
> >  #include <libcamera/timer.h>
> >  
> > -#include <iostream>
> > -
> >  #include "thread.h"
> >  #include "v4l2_videodevice_test.h"
> >  
> > diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
> > index 975c852b8cbf..42c653d4435a 100644
> > --- a/test/v4l2_videodevice/controls.cpp
> > +++ b/test/v4l2_videodevice/controls.cpp
> > @@ -5,8 +5,8 @@
> >   * controls.cpp - V4L2 device controls handling test
> >   */
> >  
> > -#include <climits>
> >  #include <iostream>
> > +#include <limits.h>
> >  
> >  #include "v4l2_videodevice.h"
> >  
> > diff --git a/test/v4l2_videodevice/formats.cpp b/test/v4l2_videodevice/formats.cpp
> > index ee7d357de075..d504d1788d02 100644
> > --- a/test/v4l2_videodevice/formats.cpp
> > +++ b/test/v4l2_videodevice/formats.cpp
> > @@ -5,8 +5,8 @@
> >   * libcamera V4L2 device format handling test
> >   */
> >  
> > -#include <climits>
> >  #include <iostream>
> > +#include <limits.h>
> >  
> >  #include "v4l2_videodevice.h"
> >

Patch

diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h
index 34e7b8ac8e87..f55fe3c09660 100644
--- a/include/libcamera/timer.h
+++ b/include/libcamera/timer.h
@@ -8,7 +8,7 @@ 
 #define __LIBCAMERA_TIMER_H__
 
 #include <chrono>
-#include <cstdint>
+#include <stdint.h>
 
 #include <libcamera/object.h>
 #include <libcamera/signal.h>
diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
index 8a939c622703..d05c01f1b13e 100644
--- a/src/cam/capture.cpp
+++ b/src/cam/capture.cpp
@@ -6,9 +6,9 @@ 
  */
 
 #include <chrono>
-#include <climits>
 #include <iomanip>
 #include <iostream>
+#include <limits.h>
 #include <sstream>
 
 #include "capture.h"
diff --git a/src/cam/options.cpp b/src/cam/options.cpp
index 7c3948df3b5c..1f0631eccfba 100644
--- a/src/cam/options.cpp
+++ b/src/cam/options.cpp
@@ -5,7 +5,7 @@ 
  * options.cpp - cam - Options parsing
  */
 
-#include <cassert>
+#include <assert.h>
 #include <getopt.h>
 #include <iomanip>
 #include <iostream>
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 570145ce71b2..9a13f5c7df17 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -6,9 +6,9 @@ 
  */
 
 #include <algorithm>
-#include <cstdint>
 #include <math.h>
 #include <queue>
+#include <stdint.h>
 #include <string.h>
 
 #include <linux/rkisp1-config.h>
diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
index ddcd59ea52c1..b2c5fd221dcd 100644
--- a/src/libcamera/device_enumerator_udev.cpp
+++ b/src/libcamera/device_enumerator_udev.cpp
@@ -8,11 +8,10 @@ 
 #include "device_enumerator_udev.h"
 
 #include <algorithm>
-#include <list>
-#include <map>
-
 #include <fcntl.h>
 #include <libudev.h>
+#include <list>
+#include <map>
 #include <string.h>
 #include <sys/ioctl.h>
 #include <sys/sysmacros.h>
diff --git a/src/libcamera/include/event_dispatcher_poll.h b/src/libcamera/include/event_dispatcher_poll.h
index d82b302c4aea..1f0738617425 100644
--- a/src/libcamera/include/event_dispatcher_poll.h
+++ b/src/libcamera/include/event_dispatcher_poll.h
@@ -7,12 +7,12 @@ 
 #ifndef __LIBCAMERA_EVENT_DISPATCHER_POLL_H__
 #define __LIBCAMERA_EVENT_DISPATCHER_POLL_H__
 
-#include <libcamera/event_dispatcher.h>
-
 #include <list>
 #include <map>
 #include <vector>
 
+#include <libcamera/event_dispatcher.h>
+
 struct pollfd;
 
 namespace libcamera {
diff --git a/src/libcamera/include/ipc_unixsocket.h b/src/libcamera/include/ipc_unixsocket.h
index 03e9fe492bde..820d05611049 100644
--- a/src/libcamera/include/ipc_unixsocket.h
+++ b/src/libcamera/include/ipc_unixsocket.h
@@ -8,7 +8,7 @@ 
 #ifndef __LIBCAMERA_IPC_UNIXSOCKET_H__
 #define __LIBCAMERA_IPC_UNIXSOCKET_H__
 
-#include <cstdint>
+#include <stdint.h>
 #include <sys/types.h>
 #include <vector>
 
diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp
index 51f9f86b4c44..50f345b98c74 100644
--- a/src/libcamera/log.cpp
+++ b/src/libcamera/log.cpp
@@ -7,14 +7,14 @@ 
 
 #include "log.h"
 
-#include <cstdio>
-#include <cstdlib>
-#include <ctime>
 #include <fstream>
 #include <iostream>
 #include <list>
+#include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <syslog.h>
+#include <time.h>
 #include <unordered_set>
 
 #include <libcamera/logging.h>
diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
index bd99deba098d..0d6630fbd1f4 100644
--- a/src/libcamera/media_device.cpp
+++ b/src/libcamera/media_device.cpp
@@ -9,11 +9,10 @@ 
 
 #include <errno.h>
 #include <fcntl.h>
+#include <string>
 #include <string.h>
 #include <sys/ioctl.h>
 #include <unistd.h>
-
-#include <string>
 #include <vector>
 
 #include <linux/media.h>
diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
index 8794ff4578c9..ef32065c161d 100644
--- a/src/libcamera/media_object.cpp
+++ b/src/libcamera/media_object.cpp
@@ -8,10 +8,9 @@ 
 #include "media_object.h"
 
 #include <errno.h>
+#include <string>
 #include <string.h>
 #include <unistd.h>
-
-#include <string>
 #include <vector>
 
 #include <linux/media.h>
diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
index 610920d1e5b3..b8e7209c1047 100644
--- a/src/libcamera/stream.cpp
+++ b/src/libcamera/stream.cpp
@@ -9,8 +9,8 @@ 
 
 #include <algorithm>
 #include <array>
-#include <climits>
 #include <iomanip>
+#include <limits.h>
 #include <sstream>
 
 #include <libcamera/request.h>
diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
index 0bdeb167a675..cafc249bbbc9 100644
--- a/test/ipa/ipa_interface_test.cpp
+++ b/test/ipa/ipa_interface_test.cpp
@@ -6,13 +6,12 @@ 
  */
 
 #include <fcntl.h>
+#include <iostream>
 #include <string.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
 
-#include <iostream>
-
 #include <libcamera/event_dispatcher.h>
 #include <libcamera/event_notifier.h>
 #include <libcamera/timer.h>
diff --git a/test/media_device/media_device_print_test.cpp b/test/media_device/media_device_print_test.cpp
index 30d929b8c763..8dd8a151266d 100644
--- a/test/media_device/media_device_print_test.cpp
+++ b/test/media_device/media_device_print_test.cpp
@@ -4,8 +4,8 @@ 
  *
  * media_device_print_test.cpp - Print out media devices
  */
-#include <iostream>
 
+#include <iostream>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
diff --git a/test/pipeline/ipu3/ipu3_pipeline_test.cpp b/test/pipeline/ipu3/ipu3_pipeline_test.cpp
index 8bfcd609a071..a5c6be0955df 100644
--- a/test/pipeline/ipu3/ipu3_pipeline_test.cpp
+++ b/test/pipeline/ipu3/ipu3_pipeline_test.cpp
@@ -6,7 +6,6 @@ 
  */
 
 #include <iostream>
-
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
diff --git a/test/v4l2_subdevice/test_formats.cpp b/test/v4l2_subdevice/test_formats.cpp
index e90c2c2426da..5cf5d5664b04 100644
--- a/test/v4l2_subdevice/test_formats.cpp
+++ b/test/v4l2_subdevice/test_formats.cpp
@@ -5,8 +5,8 @@ 
  * libcamera V4L2 Subdevice format handling test
  */
 
-#include <climits>
 #include <iostream>
+#include <limits.h>
 
 #include "v4l2_subdevice.h"
 #include "v4l2_subdevice_test.h"
diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp
index 17eb528b12fd..442a4fe56eac 100644
--- a/test/v4l2_videodevice/capture_async.cpp
+++ b/test/v4l2_videodevice/capture_async.cpp
@@ -5,12 +5,12 @@ 
  * libcamera V4L2 API tests
  */
 
+#include <iostream>
+
 #include <libcamera/buffer.h>
 #include <libcamera/event_dispatcher.h>
 #include <libcamera/timer.h>
 
-#include <iostream>
-
 #include "thread.h"
 #include "v4l2_videodevice_test.h"
 
diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
index 975c852b8cbf..42c653d4435a 100644
--- a/test/v4l2_videodevice/controls.cpp
+++ b/test/v4l2_videodevice/controls.cpp
@@ -5,8 +5,8 @@ 
  * controls.cpp - V4L2 device controls handling test
  */
 
-#include <climits>
 #include <iostream>
+#include <limits.h>
 
 #include "v4l2_videodevice.h"
 
diff --git a/test/v4l2_videodevice/formats.cpp b/test/v4l2_videodevice/formats.cpp
index ee7d357de075..d504d1788d02 100644
--- a/test/v4l2_videodevice/formats.cpp
+++ b/test/v4l2_videodevice/formats.cpp
@@ -5,8 +5,8 @@ 
  * libcamera V4L2 device format handling test
  */
 
-#include <climits>
 #include <iostream>
+#include <limits.h>
 
 #include "v4l2_videodevice.h"