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

Lja mods #517

Closed
wants to merge 6 commits into from
Closed

Lja mods #517

wants to merge 6 commits into from

Conversation

MiesSuomesta
Copy link

Some fixes that would be nice-to-have .. nothing is broken but bit better ?

@hawicz
Copy link
Member

hawicz commented Apr 21, 2020

Much of the changes here have been rendered irrelevant by the clang-format reformatting and the switch to cmake.
The stringdup() change is just broken and must not be included, plus it needlessly loses out on any platform-specific optimizations that strdup might have.
Changes like s/malloc/calloc/, and setting local variables to NULL just add pointless overhead and should not be done either.
However, the addition of some defines in printbuf.c (PRINTBUF_DEFAULT_SIZE, etc...) and the use of sizeof instead of hardcoded values could be worthwhile (though please avoid bad cases like "long unsigned int" when it should be "size_t"). If you want to re-spin that as a new PR I'll take a look at merging it.

@hawicz hawicz closed this Apr 21, 2020
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

2 participants