Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

#Improved Makefile

Improved Makefile

#Improved Makefile

Improved Makefile

Source Link
Toby Speight
  • 87.8k
  • 14
  • 104
  • 325

I'll review the Makefile here. There's a surprising amount to say, for such a short file. I'll assume your using GNU Make - if not, it's worth switching, and it's widely available.

all: clean compile run

In a parallel build, you really don't want to be cleaning as you compile (or running before compile has finished, but I'll come to that later). I recommend that you don't clean by default, as doing so negates much of the benefit of using Make.

Including run in the default target is unconventional, and will surprise others; I recommend you simply build the program as the default target, and have users make run if they want to execute it.

compile:

The rules for this target don't create the target (it makes bitmasker rather than compile), so they will be executed even when the binary is up to date. We should be honest about what it makes, and be clear when it needs remaking:

bitmasker: $(wildcard *.c)
 $(CC) -o $@ $^ -Wall -Wextra -pedantic `pkg-config --cflags --libs gtk+-3.0` -export-dynamic

Really, we don't want to be rebuilding all the sources whenever any one of them changes; instead, let's compile them to object files, and only rebuild the ones that need it:

LDLIBS += $(shell pkg-config --libs gtk+-3.0)
LDLIBS += -export-dynamic
bitmasker: main.o handlers.o utilities.o
 $(LINK.c) $^ $(LDLIBS) -o $@

If we set CFLAGS appropriately, we don't need to write rules for making *.o from *.c, because Make's built-in rule is perfect. We just need to add dependencies on the header files:

CFLAGS += -Wall -Wextra -pedantic
CFLAGS += $(shell pkg-config --cflags gtk+-3.0)
main.o: handlers.h utilities.h
handlers.o: utilities.h

(In passing, it's surprising that handlers.c doesn't include handlers.h, and utilities.c doesn't include utilities.h - is that an oversight?).

run:
 ./bitmasker

We want to be sure that the program is up to date if we want to run it:

run: bitmasker
 ./bitmasker
clean:
 rm -f bitmasker

Mostly good, though we can use the cross-platform $(RM) provided by Make, and both run and clean should be declared as "phony" rules (i.e should be made even if the target already exists).

The final piece we're missing is that we should declare .DELETE_ON_ERROR:, to prevent partially-written targets being considered up to date when a command fails or is interrupted.


#Improved Makefile

PROGRAM = bitmasker
CFLAGS += -Wall -Wextra -pedantic
CFLAGS += $(shell pkg-config --cflags gtk+-3.0)
LDLIBS += $(shell pkg-config --libs gtk+-3.0)
LDLIBS += -export-dynamic
$(PROGRAM): main.o handlers.o utilities.o
 $(LINK.c) $^ $(LDLIBS) -o $@
main.o: handlers.h utilities.h
handlers.o: utilities.h
run: $(PROGRAM)
 ./$(PROGRAM)
clean:
 $(RM) *.o $(PROGRAM)
.PHONY: clean run
.DELETE_ON_ERROR:
lang-c

AltStyle によって変換されたページ (->オリジナル) /