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

refactor: Remove 9 year old comment in memdebug.c #5973

Closed

Conversation

emilengler
Copy link
Contributor

The comment described a way how an ancient libcurl version handled
memory. 9 years later this has no value to be in there. If people need
to know about it they can have a look at VCS logs.

emilengler added a commit to emilengler/curl that referenced this pull request Sep 17, 2020
The comment described a way how an ancient libcurl version handled
memory. 9 years later this has no value to be in there. If people need
to know about it they can have a look at VCS logs.

Closes curl#5973
@emilengler emilengler force-pushed the 2020-09-refactor-memdebug-comment branch from 16693c6 to 93bd41c Compare September 17, 2020 15:15
@bagder
Copy link
Member

bagder commented Sep 17, 2020

I think you should consider removing the entire memfill functionality if you remove the backstory/docs for it.

That custom memory fill thing has lost its purpose. valgrind and ASAN are superior tools to catch memory issues.

@emilengler
Copy link
Contributor Author

I will try to work on this until Saturday. I am currently a bit busy

emilengler added a commit to emilengler/curl that referenced this pull request Sep 22, 2020
The used to be away how an ancient libcurl version handled
memory. 9 years later this has no value to be in there. If people need
to know about it they can have a look at VCS logs.

Closes curl#5973
@emilengler emilengler force-pushed the 2020-09-refactor-memdebug-comment branch from 93bd41c to bcc3fe9 Compare September 22, 2020 15:42
@emilengler
Copy link
Contributor Author

Found some time actually! This is ready for merge I think

@bagder
Copy link
Member

bagder commented Sep 22, 2020

You need to remove the use of those macros too:

memdebug.c:131:5: error: implicit declaration of function 'mt_malloc_fill' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    mt_malloc_fill(mem->mem, wantedsize);
    ^
memdebug.c:279:5: error: implicit declaration of function 'mt_free_fill' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    mt_free_fill(mem->mem, mem->size);
    ^
2 errors generated.

@emilengler
Copy link
Contributor Author

You need to remove the use of those macros too:


memdebug.c:131:5: error: implicit declaration of function 'mt_malloc_fill' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

    mt_malloc_fill(mem->mem, wantedsize);

    ^

memdebug.c:279:5: error: implicit declaration of function 'mt_free_fill' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

    mt_free_fill(mem->mem, mem->size);

    ^

2 errors generated.

Sure... but my cc haven't reported a single error on that patch 😟

@bagder
Copy link
Member

bagder commented Sep 23, 2020

Then you must've forgot to build it debug-enabled - as you can see in the CI...

The used to be away how an ancient libcurl version handled
memory. 9 years later this has no value to be in there. If people need
to know about it they can have a look at VCS logs.

Closes curl#5973
@emilengler emilengler force-pushed the 2020-09-refactor-memdebug-comment branch from bcc3fe9 to 97f3657 Compare September 28, 2020 15:31
@bagder bagder closed this in 82d66f1 Sep 28, 2020
@bagder
Copy link
Member

bagder commented Sep 28, 2020

Thanks! (I edited the commit message somewhat)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants