From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54401) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buZil-000402-JY for qemu-devel@nongnu.org; Thu, 13 Oct 2016 02:42:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buZik-0006gZ-39 for qemu-devel@nongnu.org; Thu, 13 Oct 2016 02:42:26 -0400 Date: Thu, 13 Oct 2016 12:12:08 +0530 (IST) From: P J P In-Reply-To: Message-ID: References: <1475903058-729-1-git-send-email-ppandit@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Subject: Re: [Qemu-devel] [PATCH v4] timer: a9gtimer: remove loop to auto-increment comparator List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Qemu Developers , Li Qiang , qemu-arm +-- On Wed, 12 Oct 2016, Peter Maydell wrote --+ | > - while (gtb->compare < update.new) { | > + if (gtb->compare < update.new) { | > | > + inc = update.new - gtb->compare - 1; | | Can you explain why the '- 1' here ? I think I did that because while was running till gtb->compare < update.new. To contain gtb->compare under update.new. | Something still doesn't look right here. Consider the case where | update.new is only just bigger than gtb->compare (actually the | usual case, I think). In this case 'update.new - gtb->compare - 1' | is smaller than gtb->inc and so the QEMU_ALIGN_DOWN will produce | an inc value of 0. That would be wrong, because we should definitely | have done an auto-increment. Yes, but in those cases 'gtb->compare += inc' would exceed update.new. Is that okay ? Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F