Re: [Xen-devel] [PATCH v9 24/27] xsplice: Stacking build-id dependency checking.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.04.16 at 17:35, <konrad.wilk@xxxxxxxxxx> wrote:
> @@ -25,7 +28,7 @@ clean::
>  .PHONY: config.h
>  config.h: OLD_CODE_SZ=$(call CODE_SZ,$(BASEDIR)/xen-syms,xen_extra_version)
>  config.h: NEW_CODE_SZ=$(call CODE_SZ,$<,xen_hello_world)
> -config.h: xen_hello_world_func.o
> +config.h: xen_hello_world_func.o xen_bye_world_func.o

Why is that?

> @@ -33,9 +36,43 @@ config.h: xen_hello_world_func.o
>  xen_hello_world.o: xen_hello_world_func.o
>  
>  .PHONY: $(XSPLICE)
> -$(XSPLICE): config.h xen_hello_world_func.o xen_hello_world.o
> -     $(LD) $(LDFLAGS) -r -o $(XSPLICE) xen_hello_world_func.o \
> -             xen_hello_world.o
> +$(XSPLICE): config.h xen_hello_world_func.o xen_hello_world.o note.o
> +     $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(XSPLICE) \
> +             xen_hello_world_func.o xen_hello_world.o note.o

Probably easier to read and maintain if you used $(filter %.o,$^)
here?

> +xen_bye_world.o: xen_bye_world_func.o

Again - why?

> +.PHONY: $(XSPLICE_BYE)
> +$(XSPLICE_BYE): $(XSPLICE) config.h xen_bye_world_func.o xen_bye_world.o 
> hello_world_note.o

The object files depend on config.h, but the binary does only
indirectly via the object files I would guess. (This, just like the
question right above, would then apply to the $(XSPLICE) related
rules too, in an earlier patch.)

> +     $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(XSPLICE_BYE) \
> +             xen_bye_world_func.o xen_bye_world.o hello_world_note.o

Same as above - better use $^ (and if config.h goes away as a
direct dependency, it looks like you don't even need $(filter ...)).

> +int xen_build_id_check(const Elf_Note *n, unsigned int n_sz,
> +                       const void **p, unsigned int *len)
> +{
> +    /* Check if we really have a build-id. */
> +    if ( NT_GNU_BUILD_ID != n->type )
> +        return -ENODATA;
> +
> +    if ( n_sz <= sizeof(*n) )
> +        return -EINVAL;
> +
> +    if ( n->namesz + n->descsz > UINT_MAX )

Afaict this is always false. I think you really want

    if ( n->namesz + n->descsz < n->namesz )

> +        return -EINVAL;
> +
> +    if ( n->namesz != 4 /* GNU\0 */)

< 4 would suffice here (and be more flexible if odd padding gets
inserted by whatever generates the note)

> +        return -EINVAL;
> +
> +    if ( n->namesz + n->descsz + sizeof(*n) > n_sz )

    if ( n->namesz + n->descsz > n_sz - sizeof(*n) )

> @@ -98,18 +130,9 @@ static int __init xen_build_init(void)
>      if ( &n[1] > __note_gnu_build_id_end )
>          return -ENODATA;;
>  
> -    /* Check if we really have a build-id. */
> -    if ( NT_GNU_BUILD_ID != n->type )
> -        return -ENODATA;
> +    sz = (size_t)__note_gnu_build_id_end - (size_t)n;

So let's hope sizeof(void *) == sizeof(size_t) (or else this would yield
compiler warnings).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel