From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.85.221.67; helo=mail-wr1-f67.google.com; envelope-from=philmd@redhat.com; receiver=edk2-devel@lists.01.org Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E2ECA2097F579 for ; Wed, 7 Nov 2018 01:15:12 -0800 (PST) Received: by mail-wr1-f67.google.com with SMTP id k15-v6so13562302wre.12 for ; Wed, 07 Nov 2018 01:15:12 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=L2nCfP/U4bYf4p6Xh19QrUAX8DCMwiQf8fC6U7+U9bo=; b=Xn3EfrKvyw3C0MtXufw1a8fqfDB3KOQCIaHgX8Fo292kgQqs34ldqgpWXkMNXMtEg2 0l5LZrYKO0lZO+9Sb41m4BkzJ1n2kWfkl74Tne4CtPG9vn5070s+rMLvXczKmMHZhDxG I5uwkFki6cMJH2OImX3Hp6I+QbLEsrsE0vC6XUDG7kyZIFrZSqvfqikOtnp6KrB4CYJ4 bXyEOJrj4U1FgWyD+upH+2ECNOMU3AeQnXj5PWvOtTs5jYwdNGxrwIMjjjLiukQCmNXa Zy+kl8HM+e3/3aPLy8f+Huh05by1x2YuTFknYGrUr+ySysTwOdlOHjzxHXTfyz+oP8Fz wzBA== X-Gm-Message-State: AGRZ1gI/oVQx1ipPfYUWaOhpprNvPHQi8A2vqaWG86iComj2Hjxkuvom AMZoGzQMyM+GWFYOqO0/lqkIgQ== X-Google-Smtp-Source: AJdET5ftzNHHvzSNPKlI3/r8s/HIMTCZMf7RrmBjq5O3aFwlQGSedEYriraq3S9Ho9SrfOVvwP9Fzg== X-Received: by 2002:adf:8b56:: with SMTP id v22-v6mr995399wra.33.1541582111024; Wed, 07 Nov 2018 01:15:11 -0800 (PST) Received: from ?IPv6:2a01:cb1d:8a0a:f500:48c1:8eab:256a:caf9? ([2a01:cb1d:8a0a:f500:48c1:8eab:256a:caf9]) by smtp.gmail.com with ESMTPSA id l186-v6sm1149066wma.13.2018.11.07.01.15.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 07 Nov 2018 01:15:10 -0800 (PST) To: Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , "Kinney, Michael D" , "Jin, Eric" References: <20181106175833.26964-1-ard.biesheuvel@linaro.org> <20181106175833.26964-10-ard.biesheuvel@linaro.org> <02cd4706-221e-3f21-f652-efcdc378973c@redhat.com> From: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= Message-ID: Date: Wed, 7 Nov 2018 10:08:12 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: Subject: Re: [PATCH edk2-staging 09/19] IntelUndiPkg/GigUndiDxe: add missing UINT8* cast X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Nov 2018 09:15:14 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit On 6/11/18 21:35, Ard Biesheuvel wrote: > On 6 November 2018 at 21:31, Philippe Mathieu-Daudé wrote: >> Hi Ard, >> >> On 6/11/18 18:58, Ard Biesheuvel wrote: >>> >>> UINT8 and CHAR8 are not the same underlying type on all architectures, >>> so add an explicit cast where necessary. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Ard Biesheuvel >>> --- >>> IntelUndiPkg/GigUndiDxe/Hii.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/IntelUndiPkg/GigUndiDxe/Hii.c b/IntelUndiPkg/GigUndiDxe/Hii.c >>> index a5d8ae207819..737a59fbbbac 100644 >>> --- a/IntelUndiPkg/GigUndiDxe/Hii.c >>> +++ b/IntelUndiPkg/GigUndiDxe/Hii.c >>> @@ -817,7 +817,7 @@ HiiSetMenuStrings ( >>> Status = ReadPbaString ( >>> &UndiPrivateData->NicInfo, >>> - PBAString8, >>> + (UINT8 *)PBAString8, >>> MAX_PBA_STR_LENGTH >>> ); >>> if (Status == EFI_SUCCESS) { >>> >> >> I'm not sure why ReadPbaString() takes UINT8* instead of CHAR8*. >> Having the device part number stored into a CHAR8[] seems correct, what do >> you think? > > I guess. But that just moves the bubble in the waterbed to elsewhere: > > EFI_STATUS > ReadPbaString ( > IN GIG_DRIVER_DATA *GigAdapter, > IN OUT UINT8 * PbaNumber, > IN UINT32 PbaNumberSize > ) > { > if (e1000_read_pba_string (&GigAdapter->Hw, PbaNumber, > PbaNumberSize) == E1000_SUCCESS) { > return EFI_SUCCESS; > } else { > return EFI_DEVICE_ERROR; > } > } > > and > > $ git grep e1000_read_pba_string > IntelUndiPkg/GigUndiDxe/e1000.c: if (e1000_read_pba_string > (&GigAdapter->Hw, PbaNumber, PbaNumberSize) == E1000_SUCCESS) { > IntelUndiPkg/GigUndiDxe/e1000_api.c: * e1000_read_pba_string - Read > device part number string > IntelUndiPkg/GigUndiDxe/e1000_api.c:s32 e1000_read_pba_string(struct > e1000_hw *hw, u8 *pba_num, u32 pba_num_size) > IntelUndiPkg/GigUndiDxe/e1000_api.c: return > e1000_read_pba_string_generic(hw, pba_num, pba_num_size); > IntelUndiPkg/GigUndiDxe/e1000_api.h:s32 e1000_read_pba_string(struct > e1000_hw *hw, u8 *pba_num, u32 pba_num_size); > IntelUndiPkg/GigUndiDxe/e1000_nvm.c: * e1000_read_pba_string_generic > - Read device part number > IntelUndiPkg/GigUndiDxe/e1000_nvm.c:s32 > e1000_read_pba_string_generic(struct e1000_hw *hw, u8 *pba_num, > IntelUndiPkg/GigUndiDxe/e1000_nvm.c: > DEBUGFUNC("e1000_read_pba_string_generic"); > IntelUndiPkg/GigUndiDxe/e1000_nvm.h:s32 > e1000_read_pba_string_generic(struct e1000_hw *hw, u8 *pba_num, > > (unless you want to add a cast in ReadPbaString() instead) Hmm I now see the inconsistency. Since the goal of this series is not to sort it out, then I'm OK with your patch. Reviewed-by: Philippe Mathieu-Daudé