linux-kernel - Re: [PATCH v2] kernel/power/hibernate.c: use 'u64' instead of 's64' to avoid warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 22 Apr 2014 13:29:42 +0200
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Chen Gang <gang.chen.5i5j@...il.com>
Cc:	Pavel Machek <pavel@....cz>, Guan Xuetao <gxt@...c.pku.edu.cn>,
	len.brown@...el.com,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v2] kernel/power/hibernate.c: use 'u64' instead of 's64' to avoid warning

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?

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.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/