From mboxrd@z Thu Jan  1 00:00:00 1970
From: Neil Horman <nhorman@tuxdriver.com>
Subject: Re: [PATCHv4 4/5] Makefile: Do post processing on
 objects that register a driver
Date: Wed, 25 May 2016 13:40:51 -0400
Message-ID: <20160525174051.GD14322@hmsreliant.think-freely.org>
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>
 <1591100.WzDkHNrJoU@xps13>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, Bruce Richardson <bruce.richardson@intel.com>,
 Stephen Hemminger <stephen@networkplumber.org>,
 Panu Matilainen <pmatilai@redhat.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])
 by dpdk.org (Postfix) with ESMTP id 9E5B9377C
 for <dev@dpdk.org>; Wed, 25 May 2016 19:41:00 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <1591100.WzDkHNrJoU@xps13>
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>

On Wed, May 25, 2016 at 07:08:19PM +0200, Thomas Monjalon wrote:
> 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?
> 

Looks like, I'll remove it

> >  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.
Very well.

> 
> > +		echo MODGEN $@; \
> > +		OBJF=`readlink -f $@`; \
> > +		${RTE_OUTPUT}/buildtools/pmdinfogen \$$OBJF \$$OBJF.mod.c; \
> 
> Maybe .pmd.c would be more appropriate than .mod.c?
fine
> What means mod/MODGEN/MODBUILD?
GENerate Module information & BUILD module information.

> 
> > +		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"?
Debugging statement, I'll remove it.

> 
> It deserves to be in a shell script, at least to ease testing.
What do you mean by "it" and why would it be easier to test in a shell script?

> 
>