Re: [dpdk-dev] [PATCH] kni: fixed compilation error on Ubuntu 14.04 LTS (kernel 3.13.0-30.54)

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2014-07-24 16:31, Buriez, Patrice:
> > Why not this simpler form?
> > $(shell lsb_release -si 2>/dev/null)
> 
> I didn't want "make" to stop on error or to display a warning if lsb_release is not available on other distributions.
> I must admit that I focused on identifying the exact 5-tuple UBUNTU_KERNEL_CODE that was triggering the compilation error.
> Then I tried to keep the Makefile as simple and readable as possible, and took no shortcut.
> If your simpler form works the same, then indeed it's nicer than mine. ;-)
> 
> > > +MODULE_CFLAGS += -DUBUNTU_RELEASE_CODE=$(subst .,,$(shell lsb_release -sr))
> > 
> > Or you can use | tr -d . instead of subst and keep the flow from left to right.
> 
> Agreed. I seldom use tr and didn't figure out that it would perfectly fit here. Thanks!
> 
> > > +UBUNTU_KERNEL_CODE := $(shell cut -d' ' -f2 /proc/version_signature |cut -d- -f1,2)
> >                                                                         ^
> >                                                          space missing here
> 
> I usually pipe into the next command with no space in between. So that's somewhat on purpose.
> Is your comment cosmetic or about readability?
> Or are there situations that would fail, unless the space is provided between the pipe and the command?

Yes, only cosmetic.

> > > +UBUNTU_KERNEL_CODE := $(subst -,$(comma),$(UBUNTU_KERNEL_CODE))
> > > +UBUNTU_KERNEL_CODE := $(subst .,$(comma),$(UBUNTU_KERNEL_CODE))
> > 
> > Would be simpler with | tr -d .-
> 
> Agreed again (with the $(comma) from your next email, and without the -d in order to actually translate, not delete. ;-)

Yes "tr .- $(comma)" :)

> Again, I mainly focused on extracting and transmitting the 5-tuple UBUNTU_KERNEL_CODE from shell to compiler.
> I agree this can be rewritten in nicer ways, but it works, and hopefully does not break compilation on other distributions.

Acked and applied with above modifications.

Thanks
-- 
Thomas