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

AP_Filesystem: Change the judgment null of methods that return nullptr #19925

Conversation

muramura
Copy link
Contributor

I found myself using NULL in the decision of a method that returns NULLPTR.
I believe there is a difference between NULL and NULLPTR.
I will change the decision value from NULL to NULLPTR.

Copy link
Member

@hendjoshsr71 hendjoshsr71 left a comment

Choose a reason for hiding this comment

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

Looks fine.

But you missed a few in the file. You might as well do them all at once. (I got 17 when I looked, Lines 67-80, Line 107.)

If you wanted to, there are a few style fixups you could do on a few lines since you changed the lines anyways.

@muramura muramura force-pushed the AP_Change_the_judgment_null_of_methods_that_return_nullptr branch from 39a8cae to 279c9f3 Compare January 30, 2022 01:15
@muramura
Copy link
Contributor Author

@hendjoshsr71 san
Thanks for the comment.
I agreed.
I've adapted the style.

Copy link
Member

@hendjoshsr71 hendjoshsr71 left a comment

Choose a reason for hiding this comment

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

Sorry if I was confusing. I meant there are still others spots with == NULL you could replace in this file. Using VSCode and this search == NULL I find the following still left.
Line 67
Line 69
Line 74
Line 80
Line107
Line 161

@muramura
Copy link
Contributor Author

@hendjoshsr71 sam
The lines you mentioned are as follows.
The element value of the table and the return value of the method are null.
In particular, the C language methods are NULL.
The ArduPilot uses the return value of the method to make a decision.

Line 67: file_table[] returns NULL.
Line 69: The calloc method returns NULL.
Line 74: The calloc method returns NULL.
Line 80: The malloc method returns NULL.
Line 107: file_table[] returns NULL.
Line 161: file_table[].fh returns NULL.

@hendjoshsr71
Copy link
Member

I'm not a C++ expert, but I did come across this https://stackoverflow.com/questions/53532697/c-style-when-using-c-function-in-c-should-i-check-for-null-or-nullptr

Either seems fine and what you have here is better than before :)

But lets ask @peterbarker since I've heard he is a resident C++ expert recently ;)

Copy link
Contributor

@khancyr khancyr left a comment

Choose a reason for hiding this comment

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

yep, that would be great to get rid of the NULL from the codebase.
On nice things, beside off some cpp compile time check is that it also remove a lot of noise on static analysers!

@tridge tridge merged commit e209a6e into ArduPilot:master Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants