On 04/22/2014 07:29 PM, Rafael J. Wysocki wrote: > On Tuesday, April 22, 2014 05:52:36 PM Chen Gang wrote: >> On 04/22/2014 03:56 PM, Chen Gang wrote: >>> On 04/22/2014 03:21 PM, Pavel Machek wrote: >>>> On Tue 2014-04-22 09:29:20, Chen Gang wrote: >>>>> For do_div(), it need 'u64' type, which means the outside must be sure >>>>> of 'start' is not bigger than 'stop', or it will report warning. >>>>> >>>>> Even if 'start' was really bigger than 'stop', it would print incorrect >>>>> information, but for kernel, it still can continue, so use WARN_ON() is >>>>> enough. >>>>> >>>>> The related warning (with allmodconfig for unicore32): >>>>> >>>>> CC kernel/power/hibernate.o >>>>> kernel/power/hibernate.c: In function ‘swsusp_show_speed’: >>>>> kernel/power/hibernate.c:237: warning: comparison of distinct pointer >>>>> types lacks a cast >>>>> >>>>> >>>> >>>> Certainly better, but >>>> >>>>> - s64 elapsed_centisecs64; >>>>> + u64 elapsed_centisecs64; >>>>> int centisecs; >>>>> int k; >>>>> int kps; >>>>> >>>>> elapsed_centisecs64 = timeval_to_ns(stop) - timeval_to_ns(start); >>>>> + WARN_ON((s64)elapsed_centisecs64 < 0); >> >> How about to use one line comments instead of WARN_ON()? >> >> /* >> * If "(s64)elapsed_centisecs64 < 0", it will print long elapsed >> * time, it is obvious enough to user for what went wrong. >> */ > > And will the users actually read this comment? >
For subtractions, normally, need signed number, or may let code readers notice about it. When the code readers read this comment, they will understand, and save their time to think of. Normally, it is not good to add comment within a function, except the code is not written in a normal way, and need some comments to avoid wasting readers' time. > Please just change the type to silence the warning. And you can change all of > the other local variables in that function to unsigned int when you're at it. > It is fine to me -- change all of the other local variables to unsigned int. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
