PX4 / DriverFramework

Operating system and flight stack agnostic driver framework for POSIX (Linux, NuttX, Mac OS, QNX, VxWorks).
82 stars 132 forks source link

Use C++ Standard Library header names. #151

Closed CarloWood closed 8 years ago

CarloWood commented 8 years ago

The standard C library headers are deprecated in C++ (see http://en.cppreference.com/w/cpp/header).

The reason for the deprecation being a namespace argument: in C++ we have namespaces and want everything that is part of the standard library to be declared in namespace std.

Note that this won't require extra changes to the code, as everything declared by a C header is also still declared in the global namespace. For me, it's mostly that it hurts my eyes to see a C-style header in a C++ program, but the real benefit would be that you can now make sure you get the standard function by prepending std:: to things.

Ie, you really need the <cstring> version to do crazy things like this:

int main()
{
    int foo, bar, memcpy = 0;

    {
        using std::memcpy;
        memcpy(&foo, &bar, sizeof(int));
    }
}

This is the second of several commits that prepare for a project-wide fix of this issue by means of a new script 'Tools/fix_headers.sh' (to be committed) which will not touch the include in THIS commit because it is too deep (too far away from the top of the file). Hence the need for this manual commit for this case.

bkueng commented 8 years ago

I support the changes, the current includes in PX4 FW are indeed quite a mess. Thanks a lot for your cleanup efforts! Did you also look into which includes are superfluous? I think there are quite a few of these too. http://include-what-you-use.org/ might be interesting. One thing that you should be aware of is that on NuttX we have very limited C++ STL support (basically the headers in https://bitbucket.org/nuttx/nuttx/src/937a83d9e4b0d4f27f3368b60f0ab31ccc4b173c/include/cxx/?at=master). If you include any other from these, GCC will just use the system version without telling you. It can lead to very subtle bugs.

CarloWood commented 8 years ago

@bkueng Yes, my script "knows" about C++ standard headers as follows:

STDCXX_HEADERS=$(find "$BASEDIR/NuttX/misc/uClibc++/include/uClibc++" -mindepth 1 -maxdepth 1 -type f | xargs basename -a | grep '^[a-z]*$' | xargs echo)

and about C headers:

STDC_HEADERS=$(find "$BASEDIR/NuttX/nuttx/include/cxx" -mindepth 1 -maxdepth 1 -type f | xargs basename -a | grep '^c[a-z]*$' | xargs echo)

It will not assume any other (standard) headers exist.

CarloWood commented 8 years ago

The checks failed for now because https://github.com/PX4/dspal/pull/6 needs to be merged first, it seems.

davids5 commented 8 years ago

@CarloWood - we should not be using uClibc with Nuttx

CarloWood commented 8 years ago

@davids5 My remark about that was a bit irrelevant anyway ;). The script will only change C headers, aka --> (in C++ source files) - the list of files which it gets from NuttX/nuttx/include/cxx, as @bkueng also remarked about. The mentioned list of C++ standard headers are not needed.

julianoes commented 8 years ago

@CarloWood thanks for the commit. I've applied it and set the dspal submodule correctly again.