On Thu, Feb 21, 2019 at 12:08 AM Nathan Chancellor
[off-list ref] wrote:

Clang warns:

drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c:465:2: warning:
comparison of distinct pointer types ('typeof ((rtt_avg)) *' (aka 'long
long *') and 'uint64_t *' (aka 'unsigned long long *'))
[-Wcompare-distinct-pointer-types]
        do_div(rtt_avg, 6666);
        ^~~~~~~~~~~~~~~~~~~~~
include/asm-generic/div64.h:222:28: note: expanded from macro 'do_div'
        (void)(((typeof((n)) *)0) == ((uint64_t *)0));  \
               ~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~
1 warning generated.

do_div expects an unsigned dividend. Use div_s64, which expects a signed
dividend.

Fixes: 937b10c0de68 ("iwlwifi: mvm: add debug prints for FTM")
Link: https://github.com/ClangBuiltLinux/linux/issues/372
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Nathan Chancellor <redacted>
---

v1 -> v2:

* Fix logic (as the return value of div{,64}_s64 must be used), thanks
  to Arnd for the review.

oh boy, sorry I missed that in the initial code review, thanks Arnd
for the sharp eye!
Reviewed-by: Nick Desaulniers <redacted>

Side tangent: we see this kind of difference in APIs a lot (modifying
the parameter vs returning a new value or making a copy then modifying
that) in C++ when a call site isn't passing the explicit address of
some variable or an identifier that's clearly a pointer. Ex.

int foo;
bar(foo);

Doesn't tell you whether bar mutates foo or not without looking at the
definition of bar, as it could be:
void bar(int x);
or
void bar(int& x);

I miss the convention in Ruby of using `!` suffixes on methods to
differentiate between such cases. ex:

"hello".capitalize
vs
"hello".capitalize!

both return the same value, but the one with the ! mutates the
existing object, while the one without creates a new object.  And
that's a very standard convention throughout the standard library.
Whether or not people follow that convention is always another story.

One thing I'm curious about, is "why does do_div exist?" When should I
use do_div vs div_u64 (not div_s64 as is used in this patch)?

-- 
Thanks,
~Nick Desaulniers