From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCHv4 4/5] Makefile: Do post processing on
	objects that register a driver
Date: Wed, 25 May 2016 19:08:19 +0200
Message-ID: <1591100.WzDkHNrJoU@xps13>
References: <1463431287-4551-1-git-send-email-nhorman@tuxdriver.com>
 <1464118912-19658-1-git-send-email-nhorman@tuxdriver.com>
 <1464118912-19658-5-git-send-email-nhorman@tuxdriver.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, Bruce Richardson <bruce.richardson@intel.com>,
 Stephen Hemminger <stephen@networkplumber.org>,
 Panu Matilainen <pmatilai@redhat.com>
To: Neil Horman <nhorman@tuxdriver.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f43.google.com (mail-wm0-f43.google.com [74.125.82.43])
 by dpdk.org (Postfix) with ESMTP id 1F41028FD
 for <dev@dpdk.org>; Wed, 25 May 2016 19:08:21 +0200 (CEST)
Received: by mail-wm0-f43.google.com with SMTP id n129so192855801wmn.1
 for <dev@dpdk.org>; Wed, 25 May 2016 10:08:21 -0700 (PDT)
In-Reply-To: <1464118912-19658-5-git-send-email-nhorman@tuxdriver.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

2016-05-24 15:41, Neil Horman:
> --- a/mk/internal/rte.compile-pre.mk
> +++ b/mk/internal/rte.compile-pre.mk
> @@ -80,7 +80,8 @@ C_TO_O_STR = $(subst ','\'',$(C_TO_O)) #'# fix syntax highlight
>  C_TO_O_DISP = $(if $(V),"$(C_TO_O_STR)","  HOSTCC $(@)")
>  else
>  C_TO_O = $(CC) -Wp,-MD,$(call obj2dep,$(@)).tmp $(CFLAGS) \
> -	$(CFLAGS_$(@)) $(EXTRA_CFLAGS) -o $@ -c $<
> +	 $(CFLAGS_$(@)) $(EXTRA_CFLAGS) -o $@ -c $<
> +

whitespace change?

>  C_TO_O_STR = $(subst ','\'',$(C_TO_O)) #'# fix syntax highlight
>  C_TO_O_DISP = $(if $(V),"$(C_TO_O_STR)","  CC $(@)")
>  endif
> @@ -88,10 +89,26 @@ C_TO_O_CMD = 'cmd_$@ = $(C_TO_O_STR)'
>  C_TO_O_DO = @set -e; \
>  	echo $(C_TO_O_DISP); \
>  	$(C_TO_O) && \
> +	sh -c "grep -q \"PMD_REGISTER_DRIVER(.*)\" $<; \
> +	if [ \$$? -eq 0 ]; \
> +	then \

It is preferred to keep "then" at the end of the previous line.

> +		echo MODGEN $@; \
> +		OBJF=`readlink -f $@`; \
> +		${RTE_OUTPUT}/buildtools/pmdinfogen \$$OBJF \$$OBJF.mod.c; \

Maybe .pmd.c would be more appropriate than .mod.c?
What means mod/MODGEN/MODBUILD?

> +		if [ \$$? -eq 0 ]; \
> +		then \
> +			echo MODBUILD $@; \
> +			$(CC) -c -o \$$OBJF.mod.o \$$OBJF.mod.c; \
> +			$(CROSS)ld -r -o \$$OBJF.o \$$OBJF.mod.o \$$OBJF; \
> +			mv -f \$$OBJF.o \$$OBJF; \
> +		fi; \
> +	fi; \
> +	true" && \

Why "true"?

It deserves to be in a shell script, at least to ease testing.