On 20 May 2016 at 08:24, Fu, Siyuan <[email protected]> wrote: > Hi, Ard > > > > Thanks for you reminder. I’m sorry that I don’t have the ARM build > environment, will try to setup one later. For the current build failure, do > you mean the explicit casting in attached patch could fix it? >
Yes. I tried your patch and it fixes the build on ARM Reviewed-by: Ard Biesheuvel <[email protected]> Thanks, Ard. > > > Best Regards > > Siyuan > > > > From: Ard Biesheuvel [mailto:[email protected]] > Sent: Friday, May 20, 2016 2:05 PM > To: Fu, Siyuan <[email protected]> > Cc: [email protected]; Qiu, Shumin <[email protected]>; Carsey, > Jaben <[email protected]> > Subject: Re: [edk2] [Patch] ShellPkg: Add argument to set block size for > tftp command. > > > > This patch has broken the build on ARM > > In function 'DownloadFile': > <https://ci.linaro.org/jenkins/job/leg-virt-tianocore-edk2-upstream/ws/edk2/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c>:938:22: > error: pointer targets in assignment differ in signedness > [-Werror=pointer-sign] > ReqOpt.OptionStr = "blksize"; > ^ > <https://ci.linaro.org/jenkins/job/leg-virt-tianocore-edk2-upstream/ws/edk2/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c>:939:5: > error: pointer targets in passing argument 1 of 'AsciiSPrint' differ > in signedness [-Werror=pointer-sign] > AsciiSPrint (OptBuf, sizeof (OptBuf), "%d", BlockSize); > > Please don't mix CHAR8, INT8 and UINT8: they are all distinct types, > and require explicit casting when using one in place of the other. > > Thanks, > Ard, > > > On 6 May 2016 at 19:46, Carsey, Jaben <[email protected]> wrote: >> Why write the function UintnToAscDec to convert UINTN to ascii string? >> PrintLib can do that for you. >> >> Reviewed-by: Jaben Carsey <[email protected]> >> >>> -----Original Message----- >>> From: Fu, Siyuan >>> Sent: Thursday, May 05, 2016 7:33 PM >>> To: [email protected] >>> Cc: Carsey, Jaben <[email protected]>; Qiu, Shumin >>> <[email protected]> >>> Subject: [Patch] ShellPkg: Add argument to set block size for tftp >>> command. >>> Importance: High >>> >>> TFTP block size has a big impact on the transmit performance, this patch >>> is to >>> add new argument [-s <block size>] for shell "tftp" command to configure >>> the >>> block size for file download. >>> >>> Cc: Jaben Carsey <[email protected]> >>> Cc: Shumin Qiu <[email protected]> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Fu Siyuan <[email protected]> >>> --- >>> ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c | 81 >>> +++++++++++++++++++++- >>> .../UefiShellTftpCommandLib.uni | 6 +- >>> 2 files changed, 83 insertions(+), 4 deletions(-) >>> >>> diff --git a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c >>> b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c >>> index 831b9c3..e72e6f6 100644 >>> --- a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c >>> +++ b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c >>> @@ -2,7 +2,7 @@ >>> The implementation for the 'tftp' Shell command. >>> >>> Copyright (c) 2015, ARM Ltd. All rights reserved.<BR> >>> - Copyright (c) 2015, Intel Corporation. All rights reserved. <BR> >>> + Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved. >>> <BR> >>> (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR> >>> >>> This program and the accompanying materials >>> @@ -163,6 +163,7 @@ GetFileSize ( >>> @param[in] FilePath Path of the file, Unicode encoded >>> @param[in] AsciiFilePath Path of the file, ASCII encoded >>> @param[in] FileSize Size of the file in number of bytes >>> + @param[in] BlockSize Value of the TFTP blksize option >>> @param[out] Data Address where to store the address of the >>> buffer >>> where the data of the file were downloaded >>> in >>> case of success. >>> @@ -180,6 +181,7 @@ DownloadFile ( >>> IN CONST CHAR16 *FilePath, >>> IN CONST CHAR8 *AsciiFilePath, >>> IN UINTN FileSize, >>> + IN UINT16 BlockSize, >>> OUT VOID **Data >>> ); >>> >>> @@ -223,9 +225,15 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = { >>> {L"-r", TypeValue}, >>> {L"-c", TypeValue}, >>> {L"-t", TypeValue}, >>> + {L"-s", TypeValue}, >>> {NULL , TypeMax} >>> }; >>> >>> +/// >>> +/// The default block size (512) of tftp is defined in the RFC1350. >>> +/// >>> +#define MTFTP_DEFAULT_BLKSIZE 512 >>> + >>> /** >>> Function for 'tftp' command. >>> >>> @@ -271,6 +279,7 @@ ShellCommandRunTftp ( >>> UINTN FileSize; >>> VOID *Data; >>> SHELL_FILE_HANDLE FileHandle; >>> + UINT16 BlockSize; >>> >>> ShellStatus = SHELL_INVALID_PARAMETER; >>> ProblemParam = NULL; >>> @@ -278,6 +287,7 @@ ShellCommandRunTftp ( >>> AsciiRemoteFilePath = NULL; >>> Handles = NULL; >>> FileSize = 0; >>> + BlockSize = MTFTP_DEFAULT_BLKSIZE; >>> >>> // >>> // Initialize the Shell library (we must be in non-auto-init...) >>> @@ -404,6 +414,23 @@ ShellCommandRunTftp ( >>> } >>> } >>> >>> + ValueStr = ShellCommandLineGetValue (CheckPackage, L"-s"); >>> + if (ValueStr != NULL) { >>> + if (!StringToUint16 (ValueStr, &BlockSize)) { >>> + goto Error; >>> + } >>> + // >>> + // Valid range of block size is between "8" and "65464" according to >>> RFC2348. >>> + // >>> + if (BlockSize < 8 || BlockSize > 65464) { >>> + ShellPrintHiiEx ( >>> + -1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), >>> + gShellTftpHiiHandle, L"tftp", ValueStr >>> + ); >>> + goto Error; >>> + } >>> + } >>> + >>> // >>> // Locate all MTFTP4 Service Binding protocols >>> // >>> @@ -478,7 +505,7 @@ ShellCommandRunTftp ( >>> goto NextHandle; >>> } >>> >>> - Status = DownloadFile (Mtftp4, RemoteFilePath, AsciiRemoteFilePath, >>> FileSize, &Data); >>> + Status = DownloadFile (Mtftp4, RemoteFilePath, AsciiRemoteFilePath, >>> FileSize, BlockSize, &Data); >>> if (EFI_ERROR (Status)) { >>> ShellPrintHiiEx ( >>> -1, -1, NULL, STRING_TOKEN (STR_TFTP_ERR_DOWNLOAD), >>> @@ -584,6 +611,44 @@ StringToUint16 ( >>> } >>> >>> /** >>> + This function is to convert a UINTN to a ASCII string, and return the >>> + actual length of the buffer. >>> + >>> + @param[in] Number Numeric value to be converted. >>> + @param[in] Buffer The pointer to the buffer for ASCII string. >>> + @param[in] BufferSize The maxsize of the buffer. >>> + >>> + @return Length The actual length of the ASCII string. >>> + >>> +**/ >>> +UINTN >>> +UintnToAscDec ( >>> + IN UINTN Number, >>> + IN UINT8 *Buffer, >>> + IN UINTN BufferSize >>> + ) >>> +{ >>> + UINTN Index; >>> + UINTN Length; >>> + CHAR8 TempStr[64]; >>> + >>> + Index = 63; >>> + TempStr[Index] = 0; >>> + >>> + do { >>> + Index--; >>> + TempStr[Index] = (CHAR8) ('0' + (Number % 10)); >>> + Number = (UINTN) (Number / 10); >>> + } while (Number != 0); >>> + >>> + AsciiStrCpyS ((CHAR8 *) Buffer, BufferSize, &TempStr[Index]); >>> + >>> + Length = AsciiStrLen ((CHAR8 *) Buffer); >>> + >>> + return Length; >>> +} >>> + >>> +/** >>> Get the name of the NIC. >>> >>> @param[in] ControllerHandle The network physical device handle. >>> @@ -843,6 +908,7 @@ Error : >>> @param[in] FilePath Path of the file, Unicode encoded >>> @param[in] AsciiFilePath Path of the file, ASCII encoded >>> @param[in] FileSize Size of the file in number of bytes >>> + @param[in] BlockSize Value of the TFTP blksize option >>> @param[out] Data Address where to store the address of the >>> buffer >>> where the data of the file were downloaded >>> in >>> case of success. >>> @@ -860,6 +926,7 @@ DownloadFile ( >>> IN CONST CHAR16 *FilePath, >>> IN CONST CHAR8 *AsciiFilePath, >>> IN UINTN FileSize, >>> + IN UINT16 BlockSize, >>> OUT VOID **Data >>> ) >>> { >>> @@ -868,6 +935,8 @@ DownloadFile ( >>> VOID *Buffer; >>> DOWNLOAD_CONTEXT *TftpContext; >>> EFI_MTFTP4_TOKEN Mtftp4Token; >>> + EFI_MTFTP4_OPTION ReqOpt; >>> + UINT8 OptBuf[10]; >>> >>> // Downloaded file can be large. BS.AllocatePages() is more faster >>> // than AllocatePool() and avoid fragmentation. >>> @@ -900,6 +969,14 @@ DownloadFile ( >>> Mtftp4Token.Buffer = Buffer; >>> Mtftp4Token.CheckPacket = CheckPacket; >>> Mtftp4Token.Context = (VOID*)TftpContext; >>> + if (BlockSize != MTFTP_DEFAULT_BLKSIZE) { >>> + ReqOpt.OptionStr = "blksize"; >>> + UintnToAscDec (BlockSize, OptBuf, sizeof (OptBuf)); >>> + ReqOpt.ValueStr = OptBuf; >>> + >>> + Mtftp4Token.OptionCount = 1; >>> + Mtftp4Token.OptionList = &ReqOpt; >>> + } >>> >>> ShellPrintHiiEx ( >>> -1, -1, NULL, STRING_TOKEN (STR_TFTP_DOWNLOADING), >>> diff --git >>> a/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni >>> b/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni >>> index 33a8944..a16265c 100644 >>> --- >>> a/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni >>> +++ >>> b/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni >>> @@ -1,7 +1,7 @@ >>> // /** >>> // >>> // (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR> >>> -// Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved. >>> <BR> >>> +// Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved. >>> <BR> >>> // This program and the accompanying materials >>> // are licensed and made available under the terms and conditions of the >>> BSD >>> License >>> // which accompanies this distribution. The full text of the license may >>> be >>> found at >>> @@ -50,7 +50,7 @@ >>> ".SH SYNOPSIS\r\n" >>> " \r\n" >>> "TFTP [-i interface] [-l <port>] [-r <port>] [-c <retry count>] [-t >>> <timeout>]\r\n" >>> -" host remotefilepath [localfilepath]\r\n" >>> +" [-s <block size>] host remotefilepath [localfilepath]\r\n" >>> ".SH OPTIONS\r\n" >>> " \r\n" >>> " -i interface - Specifies an adapter name, i.e., eth0.\r\n" >>> @@ -61,6 +61,8 @@ >>> " wait for a response. The default value is 6.\r\n" >>> " -t <timeout> - The number of seconds to wait for a response >>> after\r\n" >>> " sending a request packet. Default value is >>> 4s.\r\n" >>> +" -s <block size> - Specifies the TFTP blksize option as defined in >>> RFC >>> 2348.\r\n" >>> +" Valid range is between 8 and 65464, default value >>> is 512.\r\n" >>> " host - Specify TFTP Server IPv4 address.\r\n" >>> " remotefilepath - TFTP server file path to download the file.\r\n" >>> " localfilepath - Local destination file path.\r\n" >>> -- >>> 2.7.4.windows.1 >> >> _______________________________________________ >> edk2-devel mailing list >> [email protected] >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
