From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753979AbbHORHq (ORCPT ); Sat, 15 Aug 2015 13:07:46 -0400 Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]:48028 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751408AbbHORHo (ORCPT ); Sat, 15 Aug 2015 13:07:44 -0400 Date: Sat, 15 Aug 2015 19:07:38 +0200 From: Pavel Machek To: "Lee, Chun-Yi" Cc: linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org, linux-pm@vger.kernel.org, "Rafael J. Wysocki" , Matthew Garrett , Len Brown , Josh Boyer , Vojtech Pavlik , Matt Fleming , Jiri Kosina , "H. Peter Anvin" , Ingo Molnar , "Lee, Chun-Yi" Subject: Re: [PATCH v2 08/16] x86/efi: Carrying hibernation key by setup data Message-ID: <20150815170737.GA25353@Nokia-N900> References: <1439273796-25359-1-git-send-email-jlee@suse.com> <1439273796-25359-9-git-send-email-jlee@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1439273796-25359-9-git-send-email-jlee@suse.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue 2015-08-11 14:16:28, Lee, Chun-Yi wrote: > For forwarding hibernation key from EFI stub to boot kernel, this patch > allocates setup data for carrying hibernation key, size and the status > of efi operating. > > Reviewed-by: Jiri Kosina Jiri, are you sure you reviewed these? This is not really english, afaict, and efi/EFI should be spelled consistently. Could you try reviewing it again? Pointing out 10s of small bugs is quite boring... > unsigned long key_size; > unsigned long attributes; > + struct setup_data *setup_data, *hibernation_setup_data; > struct hibernation_keys *keys; > + unsigned long size = 0; > efi_status_t status; > > /* Allocate setup_data to carry keys */ > + size = sizeof(struct setup_data) + sizeof(struct hibernation_keys); > status = efi_call_early(allocate_pool, EFI_LOADER_DATA, > - sizeof(struct hibernation_keys), &keys); > + size, &hibernation_setup_data); > if (status != EFI_SUCCESS) { > efi_printk(sys_table, "Failed to alloc mem for hibernation keys\n"); > return; > } > > - memset(keys, 0, sizeof(struct hibernation_keys)); > + memset(hibernation_setup_data, 0, size); > + keys = (struct hibernation_keys *) hibernation_setup_data->data; > any chance to type stuff correctly so that casts are not neccessary? > +clean_fail: > + hibernation_setup_data->type = SETUP_HIBERNATION_KEYS; > + hibernation_setup_data->len = sizeof(struct hibernation_keys); > + hibernation_setup_data->next = 0; > + keys->hkey_status = efi_status_to_err(status); > + > + setup_data = (struct setup_data *)params->hdr.setup_data; > + while (setup_data && setup_data->next) > + setup_data = (struct setup_data *)setup_data->next; way too many casts here. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html