Re: [dpdk-dev] [PATCH v1 02/10] app/test: support resources externally linked

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

It looks a lot too much tricky to be integrated without code comments ;)
Please make a documentation effort.

2016-05-06 12:48, Jan Viktorin:
> --- a/app/test/Makefile
> +++ b/app/test/Makefile
> @@ -33,6 +33,37 @@ include $(RTE_SDK)/mk/rte.vars.mk
>  
>  ifeq ($(CONFIG_RTE_APP_TEST),y)

A comment is needed here to explain the intent of the following code.

> +ifeq ($(RTE_ARCH),arm)
> +RTE_OBJCOPY_O = elf32-littlearm
> +RTE_OBJCOPY_B = arm
> +else ifeq ($(RTE_ARCH),arm64)
> +RTE_OBJCOPY_O = elf64-littleaarch64
> +RTE_OBJCOPY_B = aarch64
> +else ifeq ($(RTE_ARCH),i686)
> +RTE_OBJCOPY_O = elf32-i386
> +RTE_OBJCOPY_B = i386
> +else ifeq ($(RTE_ARCH),x86_64)
> +RTE_OBJCOPY_O = elf64-x86-64
> +RTE_OBJCOPY_B = i386:x86-64
> +else ifeq ($(RTE_ARCH),x86_x32)
> +RTE_OBJCOPY_O = elf32-x86-64
> +RTE_OBJCOPY_B = i386:x86-64
> +else
> +$(error Unrecognized RTE_ARCH: $(RTE_ARCH))
> +endif

RTE_OBJCOPY_O could be renamed RTE_OBJCOPY_TARGET.
RTE_OBJCOPY_B could be renamed RTE_OBJCOPY_ARCH.

These definitions could be done in mk/arch/

> +
> +define resource

When defining a makefile macro, the arguments must be documented on
the previous line. Otherwise, nobody (including you) will be able
to read it without parsing the code below.

It is not a simple resource, so the name must avoid the confusion.
Why not linked_resource?

> +SRCS-y += $(1).res.o
> +$(1).res.o: $(2)
> +       $(OBJCOPY) -I binary -B $(RTE_OBJCOPY_B) -O $(RTE_OBJCOPY_O)     \
> +               --rename-section                                         \
> +                       .data=.rodata,alloc,load,data,contents,readonly  \
> +               --redefine-sym _binary__dev_stdin_start=beg_$(1)         \
> +               --redefine-sym _binary__dev_stdin_end=end_$(1)           \
> +               --redefine-sym _binary__dev_stdin_size=siz_$(1)          \
> +               /dev/stdin $$@ < $$<
> +endef

[...]

> +#define REGISTER_LINKED_RESOURCE(_n) \
> +extern const char beg_ ##_n;         \
> +extern const char end_ ##_n;         \
> +REGISTER_RESOURCE(_n, &beg_ ##_n, &end_ ##_n); \

Please explain the begin/end trick.
Thanks