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

Added malloc return check #579

Closed
wants to merge 4 commits into from
Closed

Added malloc return check #579

wants to merge 4 commits into from

Conversation

EgorWeders
Copy link
Contributor

Malloc can return null if it can't get memory from os

@krlvm
Copy link

krlvm commented Aug 20, 2024

In this case, the program should crash immediately, it should not be tolerant of this

@EgorWeders
Copy link
Contributor Author

In this case, the program should crash immediately, it should not be tolerant of this

Not really, malloc can return valid pointers on other function run. (For example if other program with huge ram impact crashed). In all cases that behavior should be processed, maybe not by false return, but exit(ERR_BAD_ALLOC).

@EgorWeders
Copy link
Contributor Author

Broke branch by accident

@krlvm
Copy link

krlvm commented Aug 21, 2024

In this case, the program should crash immediately, it should not be tolerant of this

Not really, malloc can return valid pointers on other function run. (For example if other program with huge ram impact crashed). In all cases that behavior should be processed, maybe not by false return, but exit(ERR_BAD_ALLOC).

This is wrong in any case, out of memory errors are now mixed in with the others without any indication of a real problem, and at some point in time the program, when the memory ends completely, will silently stop working. Moreover, if you look closely at where you made changes, you will notice that most of the modified functions are called at startup - lack of memory during a startup should absolutely not be tolerated, no "for example if other program with huge ram impact crashed" will help, you will never go back there.

@EgorWeders
Copy link
Contributor Author

In this case, the program should crash immediately, it should not be tolerant of this

Not really, malloc can return valid pointers on other function run. (For example if other program with huge ram impact crashed). In all cases that behavior should be processed, maybe not by false return, but exit(ERR_BAD_ALLOC).

This is wrong in any case, out of memory errors are now mixed in with the others without any indication of a real problem, and at some point in time the program, when the memory ends completely, will silently stop working. Moreover, if you look closely at where you made changes, you will notice that most of the modified functions are called at startup - lack of memory during a startup should absolutely not be tolerated, no "for example if other program with huge ram impact crashed" will help, you will never go back there.

How about other option - exit(ERR_BAD_ALLOC) or any other code to indicate memory error? Cause main problem for me is not handled malloc nulls for user in runtime.

@EgorWeders
Copy link
Contributor Author

EgorWeders commented Aug 21, 2024

notice that i changed pr due lack of github interface using experience

@EgorWeders EgorWeders mentioned this pull request Jan 14, 2025
1 task
@elfring
Copy link

elfring commented Jan 14, 2025

This is wrong in any case, out of memory errors are now mixed in with the others without any indication of a real problem, and at some point in time the program, when the memory ends completely, will silently stop working.

💭 Do you care for the advice “Do not dereference null pointers”?

@krlvm
Copy link

krlvm commented Jan 14, 2025

This is wrong in any case, out of memory errors are now mixed in with the others without any indication of a real problem, and at some point in time the program, when the memory ends completely, will silently stop working.

💭 Do you care for the advice “Do not dereference null pointers”?

It is very important if you write a piece of code that others will depend on, so you don't create unsolvable problems for the developers. If you write an application it is still important, and almost nothing less, but the handling of such errors needs to be more thought out in general, and not just an silent exit from an important procedure without any indication like proposed here, which will lead to weird behaviour from the user perspective.

@elfring
Copy link

elfring commented Jan 15, 2025

🔮 Can you get further development ideas from recommendations by Matthew Wilson?
How should a production-quality main() function look like? 🤔

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.

3 participants