From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v5 06/21] xen/arm: segregate and split GIC low level functionality Date: Thu, 19 Jun 2014 11:49:53 +0100 Message-ID: <53A2C051.9020703@linaro.org> References: <1402580192-13937-1-git-send-email-vijay.kilari@gmail.com> <1402580192-13937-7-git-send-email-vijay.kilari@gmail.com> <539A2D6A.6020509@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Vijay Kilari Cc: Ian Campbell , Stefano Stabellini , Prasun Kapoor , Vijaya Kumar K , Tim Deegan , "xen-devel@lists.xen.org" , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 06/19/2014 10:56 AM, Vijay Kilari wrote: > On Fri, Jun 13, 2014 at 4:14 AM, Julien Grall wrote: >> Hi Vijay, >> >> In my answer I prefixed every comments I made on V4 but you didn't >> address/answer by "From V4". >>> static void gic_restore_pending_irqs(struct vcpu *v) >>> { >>> - int lr = 0, lrs = nr_lrs; >>> + int lr = 0, lrs; >>> struct pending_irq *p, *t, *p_r; >>> struct list_head *inflight_r; >>> unsigned long flags; >>> + unsigned int nr_lrs = gic_hw_ops->info->nr_lrs; >>> >>> + lrs = nr_lrs; >> >> >> From V4: >> >> Hmmm ... why did you create a temporary variable nr_lrs to set lrs just >> after??? > > This function uses both global variable nr_lrs and local variable lrs. > With this patch, nr_lrs is replaced with gic_hw_ops->info->nr_lrs. > So variable nr_lrs is now made local. > Can you just do unsigned int nr_lrs = gic_hw_ops->info->nr_lrs; int lrs = nr_lrs; I found this way less confusing. -- Julien Grall