Makefile: use grep -c to avoid wc -l

PATCH In review Tommi Rantala 3 participants 8 messages Feb 5, 2017 – Feb 8, 2017 Kbuild

Share & tools

Tommi Rantala

author

2017-02-05 15:53 UTC #0 lore raw

PATCH 1/5 Makefile: use grep -c to avoid wc -l

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Makefile +1 -1

index b93fc3a..16fafe8 100644
@@ -2,7 +2,7 @@ VERSION="1.8pre"
 
 INSTALL_PREFIX ?= $(DESTDIR)
 INSTALL_PREFIX ?= $(HOME)
-NR_CPUS := $(shell grep ^processor /proc/cpuinfo | /usr/bin/wc -l)
+NR_CPUS := $(shell grep -c ^processor /proc/cpuinfo)
 
 ifeq ($(CC),"")
 CC := gcc

Signature · 1 line

2.9.3

Tommi Rantala

author

2017-02-05 15:53 UTC #1 lore raw

PATCH 2/5 Makefile: use findstring to check if we are building in development mode

---
 Makefile | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Makefile +12 -3

index 16fafe8..8088608 100644
@@ -1,5 +1,11 @@
 VERSION="1.8pre"
 
+ifeq (,$(findstring pre,$(VERSION)))
+DEVEL = 0
+else
+DEVEL = 1
+endif
+
 INSTALL_PREFIX ?= $(DESTDIR)
 INSTALL_PREFIX ?= $(HOME)
 NR_CPUS := $(shell grep -c ^processor /proc/cpuinfo)

@@ -15,10 +21,11 @@ CFLAGS += -Wall -Wextra -g -O2 -I. -Iinclude/ -Wimplicit -D_FORTIFY_SOURCE=2 -D_
 CFLAGS += $(shell if $(CC) -std=gnu11 -S -o /dev/null -xc /dev/null >/dev/null 2>&1; then echo "-std=gnu11"; else echo "-std=gnu99"; fi)
 
 # Only enabled during development, and on gcc 4.9+
+ifeq ($(DEVEL), 1)
 CPP_MAJOR := $(shell $(CPP) -dumpversion 2>&1 | cut -d'.' -f1)
 CPP_MINOR := $(shell $(CPP) -dumpversion 2>&1 | cut -d'.' -f2)
-DEVEL	:= $(shell grep VERSION Makefile | head -n1 | grep pre | wc -l)
-CFLAGS	+= $(shell if [ $(CPP_MAJOR) -eq 5 -a $(CPP_MINOR) -ge 1 -a $(DEVEL) -eq 1 ] ; then echo "-Werror"; else echo ""; fi)
+CFLAGS	+= $(shell if [ $(CPP_MAJOR) -eq 5 -a $(CPP_MINOR) -ge 1 ] ; then echo "-Werror"; else echo ""; fi)
+endif
 
 ifneq ($(SYSROOT),)
 CFLAGS += --sysroot=$(SYSROOT)

@@ -46,7 +53,9 @@ LDLIBS += -lrt
 ifneq ($(shell $(CC) -v 2>&1 | grep -c "clang"), 1)
 CFLAGS += -Wlogical-op
 CFLAGS += -Wstrict-aliasing=3
-CFLAGS += $(shell if [ $(DEVEL) -eq 0 ]; then echo "-Wno-maybe-uninitialized"; else echo ""; fi)
+ifeq ($(DEVEL), 0)
+CFLAGS += -Wno-maybe-uninitialized
+endif
 endif
 
 # Sometimes useful for debugging. more useful with clang than gcc.

Signature · 1 line

2.9.3

Tommi Rantala

author

2017-02-05 15:53 UTC #2 lore raw

PATCH 3/5 Makefile: improve build time with immediate variables

In reply to Tommi Rantala

It looks like "CFLAGS += $(shell ..." is a bad idea, as make will repeat the shell expansion again and again during the build.

Use immediate variables where we can to avoid the issue.

make -j4 before this patch:
real 0m10.774s
user 0m24.541s
sys 0m10.676s

make -j4 with this patch:
real 0m8.173s
user 0m20.817s
sys 0m7.632s

---
 Makefile | 74 +++++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 38 insertions(+), 36 deletions(-)

Makefile +38 ~~-36~~

index 8088608..8a061b1 100644
@@ -18,13 +18,15 @@ LD := $(CROSS_COMPILE)$(LD)
 
 CFLAGS += -Wall -Wextra -g -O2 -I. -Iinclude/ -Wimplicit -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D__linux__
 
-CFLAGS += $(shell if $(CC) -std=gnu11 -S -o /dev/null -xc /dev/null >/dev/null 2>&1; then echo "-std=gnu11"; else echo "-std=gnu99"; fi)
+CCSTD := $(shell if $(CC) -std=gnu11 -S -o /dev/null -xc /dev/null >/dev/null 2>&1; then echo "-std=gnu11"; else echo "-std=gnu99"; fi)
+CFLAGS += $(CCSTD)
 
 # Only enabled during development, and on gcc 4.9+
 ifeq ($(DEVEL), 1)
 CPP_MAJOR := $(shell $(CPP) -dumpversion 2>&1 | cut -d'.' -f1)
 CPP_MINOR := $(shell $(CPP) -dumpversion 2>&1 | cut -d'.' -f2)
-CFLAGS	+= $(shell if [ $(CPP_MAJOR) -eq 5 -a $(CPP_MINOR) -ge 1 ] ; then echo "-Werror"; else echo ""; fi)
+WERROR	:= $(shell if [ $(CPP_MAJOR) -eq 5 -a $(CPP_MINOR) -ge 1 ] ; then echo "-Werror"; else echo ""; fi)
+CFLAGS	+= $(WERROR)
 endif
 
 ifneq ($(SYSROOT),)

@@ -75,40 +77,40 @@ test:
 	@if [ ! -f config.h ]; then  echo "[1;31mRun configure.sh first.[0m" ; exit; fi
 
 
-MACHINE		= $(shell $(CC) -dumpmachine)
-SYSCALLS_ARCH	= $(shell case "$(MACHINE)" in \
-		  (sh*) echo syscalls/sh/*.c ;; \
-		  (ia64*) echo syscalls/ia64/*.c ;; \
-		  (ppc*|powerpc*) echo syscalls/ppc/*.c ;; \
-		  (sparc*) echo syscalls/sparc/*.c ;; \
-		  (x86_64*) echo syscalls/x86/*.c \
-				 syscalls/x86/i386/*.c \
-				 syscalls/x86/x86_64/*.c;; \
-		  (i?86*) echo syscalls/x86/*.c \
-			       syscalls/x86/i386/*.c;; \
-		  esac)
-
-HEADERS		= $(patsubst %.h,%.h,$(wildcard *.h)) $(patsubst %.h,%.h,$(wildcard syscalls/*.h)) $(patsubst %.h,%.h,$(wildcard ioctls/*.h))
-
-SRCS		= $(wildcard *.c) \
-		  $(wildcard childops/*.c) \
-		  $(wildcard fds/*.c) \
-		  $(wildcard ioctls/*.c) \
-		  $(wildcard mm/*.c) \
-		  $(wildcard net/*.c) \
-		  $(wildcard rand/*.c) \
-		  $(wildcard syscalls/*.c) \
-		  $(SYSCALLS_ARCH)
-
-OBJS		= $(sort $(patsubst %.c,%.o,$(wildcard *.c))) \
-		  $(sort $(patsubst %.c,%.o,$(wildcard childops/*.c))) \
-		  $(sort $(patsubst %.c,%.o,$(wildcard fds/*.c))) \
-		  $(sort $(patsubst %.c,%.o,$(wildcard ioctls/*.c))) \
-		  $(sort $(patsubst %.c,%.o,$(wildcard mm/*.c))) \
-		  $(sort $(patsubst %.c,%.o,$(wildcard net/*.c))) \
-		  $(sort $(patsubst %.c,%.o,$(wildcard rand/*.c))) \
-		  $(sort $(patsubst %.c,%.o,$(wildcard syscalls/*.c))) \
-		  $(sort $(patsubst %.c,%.o,$(SYSCALLS_ARCH)))
+MACHINE		:= $(shell $(CC) -dumpmachine)
+SYSCALLS_ARCH	:= $(shell case "$(MACHINE)" in \
+		   (sh*) echo syscalls/sh/*.c ;; \
+		   (ia64*) echo syscalls/ia64/*.c ;; \
+		   (ppc*|powerpc*) echo syscalls/ppc/*.c ;; \
+		   (sparc*) echo syscalls/sparc/*.c ;; \
+		   (x86_64*) echo syscalls/x86/*.c \
+				  syscalls/x86/i386/*.c \
+				  syscalls/x86/x86_64/*.c;; \
+		   (i?86*) echo syscalls/x86/*.c \
+			        syscalls/x86/i386/*.c;; \
+		   esac)
+
+HEADERS		:= $(patsubst %.h,%.h,$(wildcard *.h)) $(patsubst %.h,%.h,$(wildcard syscalls/*.h)) $(patsubst %.h,%.h,$(wildcard ioctls/*.h))
+
+SRCS		:= $(wildcard *.c) \
+		   $(wildcard childops/*.c) \
+		   $(wildcard fds/*.c) \
+		   $(wildcard ioctls/*.c) \
+		   $(wildcard mm/*.c) \
+		   $(wildcard net/*.c) \
+		   $(wildcard rand/*.c) \
+		   $(wildcard syscalls/*.c) \
+		   $(SYSCALLS_ARCH)
+
+OBJS		:= $(sort $(patsubst %.c,%.o,$(wildcard *.c))) \
+		   $(sort $(patsubst %.c,%.o,$(wildcard childops/*.c))) \
+		   $(sort $(patsubst %.c,%.o,$(wildcard fds/*.c))) \
+		   $(sort $(patsubst %.c,%.o,$(wildcard ioctls/*.c))) \
+		   $(sort $(patsubst %.c,%.o,$(wildcard mm/*.c))) \
+		   $(sort $(patsubst %.c,%.o,$(wildcard net/*.c))) \
+		   $(sort $(patsubst %.c,%.o,$(wildcard rand/*.c))) \
+		   $(sort $(patsubst %.c,%.o,$(wildcard syscalls/*.c))) \
+		   $(sort $(patsubst %.c,%.o,$(SYSCALLS_ARCH)))
 
 DEPDIR= .deps

Signature · 1 line

2.9.3

Tommi Rantala

author

2017-02-05 15:53 UTC #3 lore raw

PATCH 4/5 Makefile: -Werror also for gcc 6.x

In reply to Tommi Rantala

---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Makefile +2 -2

index 8a061b1..1fc8799 100644
@@ -21,11 +21,11 @@ CFLAGS += -Wall -Wextra -g -O2 -I. -Iinclude/ -Wimplicit -D_FORTIFY_SOURCE=2 -D_
 CCSTD := $(shell if $(CC) -std=gnu11 -S -o /dev/null -xc /dev/null >/dev/null 2>&1; then echo "-std=gnu11"; else echo "-std=gnu99"; fi)
 CFLAGS += $(CCSTD)
 
-# Only enabled during development, and on gcc 4.9+
+# -Werror only enabled during development, and on gcc 5.1+
 ifeq ($(DEVEL), 1)
 CPP_MAJOR := $(shell $(CPP) -dumpversion 2>&1 | cut -d'.' -f1)
 CPP_MINOR := $(shell $(CPP) -dumpversion 2>&1 | cut -d'.' -f2)
-WERROR	:= $(shell if [ $(CPP_MAJOR) -eq 5 -a $(CPP_MINOR) -ge 1 ] ; then echo "-Werror"; else echo ""; fi)
+WERROR	:= $(shell if [ $(CPP_MAJOR) -eq 5 -a $(CPP_MINOR) -ge 1 ] || [ $(CPP_MAJOR) -ge 6 ] ; then echo "-Werror"; else echo ""; fi)
 CFLAGS	+= $(WERROR)
 endif

Signature · 1 line

2.9.3

Tommi Rantala

author

2017-02-05 15:53 UTC #4 lore raw

PATCH 5/5 btrfs/ioctl.h is not really needed

In reply to Tommi Rantala

<linux/btrfs.h> has the ioctl definitions we want, so I see no reason for checking for <btrfs/ioctl.h>.

---
 configure      | 1 -
 ioctls/btrfs.c | 7 +++----
 2 files changed, 3 insertions(+), 5 deletions(-)

configure +0 -1

index dc09317..dff66d3 100755
@@ -255,7 +255,6 @@ check_header linux/if_alg.h USE_IF_ALG
 check_header linux/rds.h USE_RDS
 check_header linux/vfio.h USE_VFIO
 check_header linux/btrfs.h USE_BTRFS
-check_header btrfs/ioctl.h USE_BTRFS_IOCTL
 check_header drm/drm.h USE_DRM
 check_header drm/exynos_drm.h USE_DRM_EXYNOS
 check_header sound/compress_offload.h USE_SNDDRV_COMPRESS_OFFLOAD

ioctls/btrfs.c +3 -4

index 374baa6..fd9c4b4 100644
@@ -1,9 +1,8 @@
 #include "config.h"
-#ifdef USE_BTRFS_IOCTL
+#ifdef USE_BTRFS
 #include <stdio.h>
 #include <linux/fs.h>
-
-#include <btrfs/ioctl.h>
+#include <linux/btrfs.h>
 #include "ioctls.h"
 #include "shm.h"
 #include "utils.h"

@@ -122,4 +121,4 @@ static const struct ioctl_group btrfs_grp = {
 };
 
 REG_IOCTL_GROUP(btrfs_grp)
-#endif /* USE_BTRFS_IOCTL */
+#endif /* USE_BTRFS */

Signature · 1 line

2.9.3

Michael Ellerman

2017-02-07 10:05 UTC #5 lore raw

In reply to Tommi Rantala

Quoted 9 lines · Tommi Rantala

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Makefile +0 -0

index b93fc3a..16fafe8 100644

Makefile VERSION="1.8pre" · line 5

Quoted 5 lines

Makefile +1 -1

@@ -2,7 +2,7 @@ VERSION="1.8pre"
 
 INSTALL_PREFIX ?= $(DESTDIR)
 INSTALL_PREFIX ?= $(HOME)
-NR_CPUS := $(shell grep ^processor /proc/cpuinfo | /usr/bin/wc -l)
+NR_CPUS := $(shell grep -c ^processor /proc/cpuinfo)

Or you could just use nproc(1) ?

It's in coreutils, but maybe that's not sufficiently portable?

cheers

Dave Jones

2017-02-07 14:14 UTC #6 lore raw

Quoted 12 lines · Michael Ellerman

Quoted 10 lines · Tommi Rantala

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Makefile +0 -0

index b93fc3a..16fafe8 100644
@@ -2,7 +2,7 @@ VERSION="1.8pre"

Quoted 6 lines

Makefile VERSION="1.8pre" · line 5
Quoted 4 lines
Makefile +1 -1
 INSTALL_PREFIX ?= $(DESTDIR)
 INSTALL_PREFIX ?= $(HOME)
-NR_CPUS := $(shell grep ^processor /proc/cpuinfo | /usr/bin/wc -l)
+NR_CPUS := $(shell grep -c ^processor /proc/cpuinfo)
Or you could just use nproc(1) ?

Is that in ancient coreutils from crusty old enterprise distros ? Judging by user reports, I think we need to support back as far as RHEL6 for now.

Quoted 1 line

It's in coreutils, but maybe that's not sufficiently portable?

Portability to non-Linux OS's hasn't been a concern so far. I had 1-2 people email me about BSD support but the patches they've sent have been about 1% of the work actually necessary so I've just rejected them.

It's way easier to just extend iknowthis (or possibly syzkaller) than to retrofit it into Trinity at this point.

I admit to having a morbid curiousity to know just how well the various BSD's would hold up under a targetted fuzzing run, but the likelyhood of me finding time to work on it, or even support it, is practically nil.

Dave

Michael Ellerman

2017-02-08 03:52 UTC #7 lore raw

Quoted 15 lines · Dave Jones

Quoted 7 lines · Michael Ellerman
Quoted 6 lines · Tommi Rantala
Makefile +0 -0
index b93fc3a..16fafe8 100644
@@ -2,7 +2,7 @@ VERSION="1.8pre"
 
Quoted 6 lines
Makefile VERSION="1.8pre" · line 5
Quoted 4 lines
Makefile +1 -1
 INSTALL_PREFIX ?= $(DESTDIR)
 INSTALL_PREFIX ?= $(HOME)
-NR_CPUS := $(shell grep ^processor /proc/cpuinfo | /usr/bin/wc -l)
+NR_CPUS := $(shell grep -c ^processor /proc/cpuinfo)
Or you could just use nproc(1) ?

Quoted 3 lines

Is that in ancient coreutils from crusty old enterprise distros ? Judging by user reports, I think we need to support back as far as RHEL6 for now.

Good point. It's there on my RHEL 6.8 box, which is the oldest I have lying around, and looks like it first existed in ~2009. But may not be worth the potential breakage.

Quoted 7 lines

Quoted 1 line
It's in coreutils, but maybe that's not sufficiently portable?
Portability to non-Linux OS's hasn't been a concern so far. I had 1-2 people email me about BSD support but the patches they've sent have been about 1% of the work actually necessary so I've just rejected them.

Quoted 6 lines

It's way easier to just extend iknowthis (or possibly syzkaller) than to retrofit it into Trinity at this point.
I admit to having a morbid curiousity to know just how well the various BSD's would hold up under a targetted fuzzing run, but the likelyhood of me finding time to work on it, or even support it, is practically nil.

I know the feeling, there's another operating system it would be fun to run on too, but likewise I will never get the time :)

cheers