Re: gcc warning in my trace_benchmark() code

[David Daney <ddaney.cavm@gmail.com>]

On 06/05/2014 10:35 AM, Steven Rostedt wrote:
> On Thu, 05 Jun 2014 10:12:16 -0700
> David Daney <ddaney.cavm@gmail.com> wrote:
>
>> On 06/05/2014 09:12 AM, Steven Rostedt wrote:
>>> I'm going through some of the warnings that Fengguang Wu's test bot has
>>> discovered, and one of them is from a MIPS allmodconfig build.
>>>
>>> https://lists.01.org/pipermail/kbuild-all/2014-May/004751.html
>>>
>>>      kernel/trace/trace_benchmark.c: In function 'trace_do_benchmark':
>>>>> kernel/trace/trace_benchmark.c:84:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
>>>>> kernel/trace/trace_benchmark.c:85:3: warning: comparison of distinct pointer types lacks a cast [enabled by default]
>>>      kernel/trace/trace_benchmark.c:38:6: warning: unused variable 'seedsq' [-Wunused-variable]
>>>
>>> vim +84 kernel/trace/trace_benchmark.c
>>>
>>>       78		if (bm_cnt > 1) {
>>>       79			/*
>>>       80			 * Apply Welford's method to calculate standard deviation:
>>>       81			 * s^2 = 1 / (n * (n-1)) * (n * \Sum (x_i)^2 - (\Sum x_i)^2)
>>>       82			 */
>>>       83			stddev = (u64)bm_cnt * bm_totalsq - bm_total * bm_total;
>>>     > 84			do_div(stddev, bm_cnt);
>>>     > 85			do_div(stddev, bm_cnt - 1);
>>>       86		} else
>>>       87			stddev = 0;
>>>       88	
>>>
>>>
>>>
>>> Is there something special with do_div in mips that I should be aware
>>> of?
>>
>> Yes.  MIPS is using the implementation in asm-generic/div64.h,  which
>> per the comments in that file has a useless pointer compare to find just
>> this type of issue.
>
> You mean this comment?
>
> /* The unnecessary pointer compare is there
>   * to check for type safety (n must be 64bit)
>   */
>

Yes.

> But stddev is s64. Ah, but the compare is:
>
> (void)(((typeof((n)) *)0) == ((uint64_t *)0));
>
> so it's complaining about a signed verses unsigned compare, not length.
> I think I can ignore this warning then.

The pedant in me thinks that you should fix your code if using do_div() 
on a signed object is undefined.  But if you aren't planning on merging 
the code, then it probably doesn't matter.

>
> Thoughts?

I think I will have lunch now...

David Daney

>