Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global: change from NULL to nullptr #12648

Closed

Conversation

muramura
Copy link
Contributor

For NULL pointers, nullptr is more appropriate for C++.

@muramura muramura changed the title Global: change from null to nullptr Global: change from NULL to nullptr Oct 24, 2019
Copy link
Contributor

@WickedShell WickedShell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't manually change the generated output for Lua.

@muramura
Copy link
Contributor Author

@WickedShell san. Comments Thanks.
I change the generator file.

@WickedShell
Copy link
Contributor

@muramura I'm not actually fully sure it's the correct change for the Lua code either way though, as the structure is getting passed to C code, and the sentinel is supposed to just be {NULL, NULL}. I'm not upto date enough with nullptr to speak to the value of that.

@muramura
Copy link
Contributor Author

I think it is better to use nullptr defined in C ++ source to solve the NULL problem.

@khancyr khancyr force-pushed the AP_Change_from_NULL_to_nullptr branch from 0856366 to b3e4524 Compare April 1, 2020 16:28
@khancyr khancyr dismissed WickedShell’s stale review April 1, 2020 16:29

Reworked and remove change on Lua files

@rmackay9
Copy link
Contributor

rmackay9 commented Apr 6, 2020

I've removed the dev-call-topic.

I suspect this hasn't been tested. I'm afraid that all changes need to be tested so we can be sure they don't break anything. Even NFC. If that's not possible, let's close this.

@khancyr
Copy link
Contributor

khancyr commented Apr 7, 2020

a flight test should be enough to validate this.
The change from NULL to nullptr ensure us that the compiler will do the right thing with pointer and not use the value 0.
The analysis is already done by the compiler since nullptr is a pointer type and cannot be used with other litteral type such as int or float. Therefore, this change are checked at compile time and unsure that we are trully speaking of pointer and not need the 0 value ! I will provide a flight log next week

@khancyr khancyr self-assigned this Apr 7, 2020
@peterbarker
Copy link
Contributor

A flight test isn't enough to validate this. We don't cross all of the relevant codepaths in a single flight.

This is now conflicting horribly, and it will probably be easier to redo from scratch. However, I suggest a better way forward would be to just stay vigilant about eliminating NULL from the code in new PRs; since this was last rebased about 12% of the NULLs have died a natural death (rough grepping done).

I'm going to close this one as a lost cause.

I'm not convinced a similar PR will gain much traction - it doesn't buy us that much. That being said, I'll look at any against AP_Logger and replay - they're some of the main offenders.

@peterbarker peterbarker closed this Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants