Skip to content

Xalloc dedup&cleanup#155

Merged
natoscott merged 3 commits intohtop-dev:masterfrom
BenBE:xalloc_dedup
Oct 5, 2020
Merged

Xalloc dedup&cleanup#155
natoscott merged 3 commits intohtop-dev:masterfrom
BenBE:xalloc_dedup

Conversation

@BenBE
Copy link
Copy Markdown
Member

@BenBE BenBE commented Sep 18, 2020

This cleans up some of the mess in XAlloc.h

This also fixes an argument list issue the original macro had (buf was missing and wrongly labelled fmt).

Copy link
Copy Markdown
Member

@fasterit fasterit left a comment

Choose a reason for hiding this comment

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

Thank you very much for cleaning up. Much appreciated!

Can we have these in StringUtils please?
Nobody would expect them in XAlloc.

@BenBE
Copy link
Copy Markdown
Member Author

BenBE commented Sep 18, 2020

Given the naming I somewhat expected them to be there. Inside XAlloc.* all the functions start with a lowercase x. For those functions to properly fit the naming for StringUtils.c/.h They should be named String_asnprintf, String_snprintf and String_strdup instead (to stay consistent).

I could rename and move them over to StringUtils.c/.h, but doing so is better suited in a separate PR IMHO.

@BenBE
Copy link
Copy Markdown
Member Author

BenBE commented Sep 19, 2020

Thank you very much for cleaning up. Much appreciated!

Can we have these in StringUtils please?
Nobody would expect them in XAlloc.

I did a dryrun with just moving these functions (without rename) and that patch already causes quite a disruption. And with proper renaming (which IMHO should be done when moving these functions over to StringUtils.h) causes not just about 20 files, but nearly 100 that would need changes; across all platforms. Furthermore I'd have to redo quite a bit of #156 and #160; thus moving those functions is best done after #160 (which also allows to better clean up after this change).

Thus I'd prefer to go along with #155 as-is for now.

@BenBE BenBE mentioned this pull request Sep 19, 2020
@BenBE BenBE force-pushed the xalloc_dedup branch 3 times, most recently from f4974d5 to 86f5f13 Compare September 28, 2020 22:01
This issue was previously hidden as xSnprintf expanded to only one large command that didn't trigger the GCC formatting check.
@natoscott natoscott merged commit dac1e05 into htop-dev:master Oct 5, 2020
@BenBE BenBE deleted the xalloc_dedup branch October 5, 2020 19:55
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