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

Consider LDFLAGS while linking #4

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mbargull
Copy link

@mbargull mbargull commented Mar 4, 2021

Adds LDFLAGS to the linking step.

A couple of further suggestions/questions:

  1. With the current change the link arguments are $(LDFLAGS) -L..
    An alternative could be to have LDFLAGS+=.L and then use $(LDFLAGS) afterwards.
  2. There are these conditional LIBS+=-fsanitize=* lines.
    Those got me a little confused; these should be compiler flags and having them in CFLAGS should suffice, shouldn't it?
  3. How do you feel about changing CPPFLAGS=-DHAVE_KALLOC to CPPFLAGS+=-DHAVE_KALLOC?
    This way CPPFLAGS from the environment can be reused but the user still has the possibility to override via make CPPFLAGS=....
  4. (Less important) How do you feel about changing CFLAGS=... to CFLAGS?=...?
    With that, custom CFLAGS don't have to be explicitly passed on as make arguments.

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.

1 participant