On 18 August 2017 at 09:26, Ingo Molnar <mi...@kernel.org> wrote:
>
> * Ard Biesheuvel <ard.biesheu...@linaro.org> wrote:
>
>> -static void for_each_tracepoint_range(struct tracepoint * const *begin,
>> -             struct tracepoint * const *end,
>> +static void for_each_tracepoint_range(const void *begin, const void *end,
>>               void (*fct)(struct tracepoint *tp, void *priv),
>>               void *priv)
>>  {
>> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>> +     const signed int *iter;
>> +
>> +     if (!begin)
>> +             return;
>> +     for (iter = begin; iter < (signed int *)end; iter++) {
>> +             fct((struct tracepoint *)((unsigned long)iter + *iter), priv);
>> +     }
>
> I think checkpatch is correct here to complain about the unnecessary curly 
> braces
> here.
>

Fair enough. I will clean up to the extent feasible.

> Plus why the heavy use of 'signed int' here and elsewhere in the patches - why
> not 'int' ?
>

Yes, just 'int' should be sufficient. Force of habit, I suppose, given
that unqualified 'char' is ambiguous between architectures, but this
does not apply to 'int' of course.

> Plus #2, the heavy use of type casts looks pretty ugly to begin with - is 
> there no
> way to turn this into more natural code?
>

Actually, Steven requested to keep the tracepoint section markers as
they are, and cast them to (int *) in the conditionally compiled
PREL32 case. So I guess it is a matter of taste, but because the types
are fundamentally incompatible when this code is in effect (and the
size of the pointed-to type differs on 64-bit architectures), there is
always some mangling required.

The initcall patch is probably the cleanest in this regard, but
involves typedefs, which are frowned upon as well.

Reply via email to