Rework memory error handling and enable debug mode by default#115
Rework memory error handling and enable debug mode by default#115cgzones wants to merge 1 commit intohtop-dev:masterfrom
Conversation
|
LTGM so far … |
fasterit
left a comment
fasterit
left a comment
There was a problem hiding this comment.
Why do you remove the inlines? Have you checked on the performance impact?
I'm not sure we want to invert the debug logic as dozens of distros need to adjust their packaging then and - typically - if you run make on what you just grabbed from git, you'd not expect a debug build either.
cgzones
added
code quality ♻️
|
The Regarding |
It tells the compiler you want it inlined and sane compilers will obey.
Similar to the -pedantic above, you drop standards in the code base to modernize it just in the place that you touch. I think this needs a more general approach to either stay with the current style / language level or go beyond that to some defined level.
I do not see why that is an argument for you to invert the logic of the code base and put work on every downstream packager. |
|
Unless there's a really good reason to do so (and I currently see none) I'd prefer to avoid any GNU extensions and thus keep For the |
cgzones
force-pushed
the
failures
branch
3 times, most recently
from
a2628f0 to
b90dd26
Compare
XAlloc.h
Outdated
| #endif | ||
| #define xAsprintf(strp, fmt, ...) \ | ||
| do { \ | ||
| int _ret = asprintf(strp, fmt, __VA_ARGS__); \ |
There was a problem hiding this comment.
Prefer vasprintf as shown with my implementation in #155.
There was a problem hiding this comment.
There is no need inside a macro for manual argument unpacking
XAlloc.h
Outdated
| } while(0) | ||
|
|
||
| char* xStrdup_(const char* str) ATTR_NONNULL; | ||
| #define xSnprintf(fmt, len, ...) \ |
There was a problem hiding this comment.
Zero sized variable arguments are a C extension, so #define xSnprintf(buf, len, fmt, ...) does not work in case of xSnprintf(foo, 42, "Hello World");
There was a problem hiding this comment.
That's not what #155 uses (partly for that reason).
Also, the current macro calls the first argument fmt, while it should be buf.
BTW: That naming issue was what led me to rewrite the macro as a proper function.
Print exact location on a memory allocation or write (snprintf) failure,
instead of just exiting with: `htop: Success`
Failed to write memory (5/7) [htop.c:211]
Aborted
|
Superseded by #204 |
3 participants
Print exact location on a memory allocation or write (snprintf) failure,
instead of just exiting with:
htop: SuccessEnable asserts by not using
-DNDEBUGby default.Let users/package maintainers specify it if they want to.
Closes: #112