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

Codechange: Use null pointer literal instead of the NULL macro #7407

Merged
merged 1 commit into from Apr 10, 2019

Conversation

@M3Henry
Copy link
Contributor

M3Henry commented Mar 24, 2019

Use of nullptr is preferential to the NULL macro since C++11.

This was done with a simple sed

find src/ -name '*.[hc]' | xargs sed -i -e 's/\bNULL\b/nullptr/g'
find src/ -name '*.[hc]pp' | xargs sed -i -e 's/\bNULL\b/nullptr/g'
@TrueBrain

This comment has been minimized.

Copy link
Member

TrueBrain commented Mar 24, 2019

a) you killed my browser. Mean! :P
b) does this prevent using NULL in the future, or how are we going to do this? Only converting it is just begging for new NULLs to sneak in, and we have to repeat this every N days :D

@M3Henry

This comment has been minimized.

Copy link
Contributor Author

M3Henry commented Mar 24, 2019

NULL should not be allowed in new code after this.

@TrueBrain

This comment has been minimized.

Copy link
Member

TrueBrain commented Mar 24, 2019

What I meant to say: any way to enforce that? Compiler-flag? Or do we need to add this to the commit-checker? :)

@Eddi-z

This comment has been minimized.

Copy link
Contributor

Eddi-z commented Mar 24, 2019

##[error]*** b/src/stdafx.h:180: Preprocessor hash is put into the first column, before the tab indentation: ' #pragma warning(disable: 6011) // code analyzer: Dereferencing nullptr pointer 'pfGetAddrInfo': Lines: 995, 996, 998, 999, 1001'

doesn't sound right

@TrueBrain

This comment has been minimized.

Copy link
Member

TrueBrain commented Mar 24, 2019

##[error]*** b/src/stdafx.h:180: Preprocessor hash is put into the first column, before the tab indentation: ' #pragma warning(disable: 6011) // code analyzer: Dereferencing nullptr pointer 'pfGetAddrInfo': Lines: 995, 996, 998, 999, 1001'

doesn't sound right

This is our commit checker being paranoid. It is a new style we enforce. But for these patches we might want to make an exception for them :) I would vote we just overrule its outcome.

@M3Henry M3Henry force-pushed the M3Henry:nullptr branch 2 times, most recently from d437a7e to ec98e36 Mar 24, 2019
@M3Henry M3Henry closed this Mar 26, 2019
@M3Henry M3Henry force-pushed the M3Henry:nullptr branch from ec98e36 to 03ca319 Mar 26, 2019
@M3Henry M3Henry reopened this Mar 26, 2019
@M3Henry M3Henry force-pushed the M3Henry:nullptr branch 2 times, most recently from c88a121 to b9c95de Mar 26, 2019
@M3Henry

This comment has been minimized.

Copy link
Contributor Author

M3Henry commented Apr 1, 2019

I have a script which I run periodically to keep this PR up to date. Otherwise there's not much left to do on this one.

find . ! -path './3rdparty/*' -name '*.h' | xargs sed -i -e 's/\bNULL\b/nullptr/g'
find . ! -path './3rdparty/*' -name '*.[hc]pp' | xargs sed -i -e 's/\bNULL\b/nullptr/g'
sed -i -e 's/hIMC != nullptr/hIMC != NULL/' video/win32_v.cpp
@M3Henry M3Henry force-pushed the M3Henry:nullptr branch from b9c95de to bbf1d65 Apr 9, 2019
@michicc

This comment has been minimized.

Copy link
Member

michicc commented Apr 9, 2019

You either should drop the comment change in stdafx.h or reformat the whole block to conform to the new coding style (#<indent>pragma instead of <indent>#pragma).

Alternatively, you find an admin to force a merge ignoring the commit-checker. I would merge, but I'm not an admin.

@M3Henry M3Henry force-pushed the M3Henry:nullptr branch from bbf1d65 to a938cfb Apr 10, 2019
@michicc michicc merged commit 7c8e7c6 into OpenTTD:master Apr 10, 2019
8 checks passed
8 checks passed
OpenTTD CI Build #20190410.10 succeeded
Details
OpenTTD CI (Linux commit-checker) Linux commit-checker succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.8) Linux linux-amd64-clang-3.8 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.