Reviewed-by: Jaben Carsey <jaben.car...@intel.com>
> -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Jiaxin Wu > Sent: Wednesday, December 02, 2015 12:54 AM > To: edk2-devel@lists.01.org > Cc: Carsey, Jaben <jaben.car...@intel.com>; Ye, Ting <ting...@intel.com>; > Cohen, Eugene <eug...@hp.com> > Subject: [edk2] [Patch] ShellPkg: Fix wrong return status for Ifconfig.c > Importance: High > > The Ifconfig command handler tries to return an EFI_STATUS when > the return type should be SHELL_STATUS. > > Cc: Cohen, Eugene <eug...@hp.com> > Cc: Carsey, Jaben <jaben.car...@intel.com> > Cc: Ye Ting <ting...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiaxin Wu <jiaxin...@intel.com> > --- > .../UefiShellNetwork1CommandsLib/Ifconfig.c | 102 ++++++++++++++---- > --- > 1 file changed, 69 insertions(+), 33 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c > index e16d46a..fb6f575 100644 > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c > @@ -421,11 +421,11 @@ IfConfigGetInterfaceInfo ( > NULL, > &HandleNum, > &HandleBuffer > ); > if (EFI_ERROR (Status) || (HandleNum == 0)) { > - return EFI_ABORTED; > + return Status; > } > > // > // Enumerate all handles that installed with ip4 service binding protocol. > // > @@ -585,15 +585,15 @@ ON_ERROR: > /** > The list process of the ifconfig command. > > @param[in] IfList The pointer of IfList(interface list). > > - @retval EFI_SUCCESS The ifconfig command list processed successfully. > + @retval SHELL_SUCCESS The ifconfig command list processed successfully. > @retval others The ifconfig command list process failed. > > **/ > -EFI_STATUS > +SHELL_STATUS > IfConfigShowInterfaceInfo ( > IN LIST_ENTRY *IfList > ) > { > LIST_ENTRY *Entry; > @@ -781,35 +781,37 @@ IfConfigShowInterfaceInfo ( > } > } > > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_BREAK), > gShellNetwork1HiiHandle); > > - return EFI_SUCCESS; > + return SHELL_SUCCESS; > } > > /** > The clean process of the ifconfig command to clear interface info. > > @param[in] IfList The pointer of IfList(interface list). > > - @retval EFI_SUCCESS The ifconfig command clean processed successfully. > + @retval SHELL_SUCCESS The ifconfig command clean processed > successfully. > @retval others The ifconfig command clean process failed. > > **/ > -EFI_STATUS > +SHELL_STATUS > IfConfigClearInterfaceInfo ( > IN LIST_ENTRY *IfList > ) > { > - EFI_STATUS Status; > + EFI_STATUS Status; > + SHELL_STATUS ShellStatus; > LIST_ENTRY *Entry; > LIST_ENTRY *Next; > IFCONFIG_INTERFACE_CB *IfCb; > EFI_IP4_CONFIG2_POLICY Policy; > > Policy = Ip4Config2PolicyDhcp; > Status = EFI_SUCCESS; > + ShellStatus = SHELL_SUCCESS; > > if (IsListEmpty (IfList)) { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN > (STR_IFCONFIG_INVALID_INTERFACE), gShellNetwork1HiiHandle); > } > > @@ -823,37 +825,37 @@ IfConfigClearInterfaceInfo ( > IfCb->IfCfg, > Ip4Config2DataTypePolicy, > sizeof (EFI_IP4_CONFIG2_POLICY), > &Policy > ); > - > if (EFI_ERROR (Status)) { > + ShellStatus = SHELL_ACCESS_DENIED; > break; > } > } > > - return Status; > + return ShellStatus; > } > > /** > The set process of the ifconfig command. > > @param[in] IfList The pointer of IfList(interface list). > @param[in] VarArg The pointer of ARG_LIST(Args with "-s" option). > > - @retval EFI_SUCCESS The ifconfig command set processed successfully. > + @retval SHELL_SUCCESS The ifconfig command set processed successfully. > @retval others The ifconfig command set process failed. > > **/ > -EFI_STATUS > +SHELL_STATUS > IfConfigSetInterfaceInfo ( > IN LIST_ENTRY *IfList, > IN ARG_LIST *VarArg > ) > { > - > EFI_STATUS Status; > + SHELL_STATUS ShellStatus; > IFCONFIG_INTERFACE_CB *IfCb; > VAR_CHECK_CODE CheckCode; > EFI_EVENT TimeOutEvt; > EFI_EVENT MappedEvt; > BOOLEAN IsAddressOk; > @@ -870,18 +872,19 @@ IfConfigSetInterfaceInfo ( > > Dns = NULL; > > if (IsListEmpty (IfList)) { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN > (STR_IFCONFIG_INVALID_INTERFACE), gShellNetwork1HiiHandle); > - return EFI_INVALID_PARAMETER; > + return SHELL_INVALID_PARAMETER; > } > > // > // Make sure to set only one interface each time. > // > IfCb = NET_LIST_USER_STRUCT (IfList->ForwardLink, > IFCONFIG_INTERFACE_CB, Link); > Status = EFI_SUCCESS; > + ShellStatus = SHELL_SUCCESS; > > // > // Initialize check list mechanism. > // > CheckCode = IfConfigRetriveCheckListByName( > @@ -899,10 +902,11 @@ IfConfigSetInterfaceInfo ( > NULL, > NULL, > &TimeOutEvt > ); > if (EFI_ERROR (Status)) { > + ShellStatus = SHELL_ACCESS_DENIED; > goto ON_EXIT; > } > > Status = gBS->CreateEvent ( > EVT_NOTIFY_SIGNAL, > @@ -910,10 +914,11 @@ IfConfigSetInterfaceInfo ( > IfConfigManualAddressNotify, > &IsAddressOk, > &MappedEvt > ); > if (EFI_ERROR (Status)) { > + ShellStatus = SHELL_ACCESS_DENIED; > goto ON_EXIT; > } > > // > // Parse the setting variables. > @@ -967,10 +972,11 @@ IfConfigSetInterfaceInfo ( > // > if (StrCmp(VarArg->Arg, L"dhcp") == 0) { > if (IfCb->Policy == Ip4Config2PolicyDhcp) { > Status = IfConfigStartIp4 (IfCb->NicHandle, gImageHandle); > if (EFI_ERROR(Status)) { > + ShellStatus = SHELL_ACCESS_DENIED; > goto ON_EXIT; > } > } else { > // > // Set dhcp config policy > @@ -981,10 +987,11 @@ IfConfigSetInterfaceInfo ( > Ip4Config2DataTypePolicy, > sizeof (EFI_IP4_CONFIG2_POLICY), > &Policy > ); > if (EFI_ERROR(Status)) { > + ShellStatus = SHELL_ACCESS_DENIED; > goto ON_EXIT; > } > } > > VarArg= VarArg->Next; > @@ -998,12 +1005,12 @@ IfConfigSetInterfaceInfo ( > IfCb->IfCfg, > Ip4Config2DataTypePolicy, > sizeof (EFI_IP4_CONFIG2_POLICY), > &Policy > ); > - > if (EFI_ERROR(Status)) { > + ShellStatus = SHELL_ACCESS_DENIED; > goto ON_EXIT; > } > > VarArg= VarArg->Next; > > @@ -1012,28 +1019,31 @@ IfConfigSetInterfaceInfo ( > // > // Get manual IP address. > // > Status = NetLibStrToIp4 (VarArg->Arg, &ManualAddress.Address); > if (EFI_ERROR(Status)) { > + ShellStatus = SHELL_INVALID_PARAMETER; > goto ON_EXIT; > } > > // > // Get subnetmask. > // > VarArg = VarArg->Next; > Status = NetLibStrToIp4 (VarArg->Arg, &ManualAddress.SubnetMask); > if (EFI_ERROR(Status)) { > + ShellStatus = SHELL_INVALID_PARAMETER; > goto ON_EXIT; > } > > // > // Get gateway. > // > VarArg = VarArg->Next; > Status = NetLibStrToIp4 (VarArg->Arg, &Gateway); > if (EFI_ERROR(Status)) { > + ShellStatus = SHELL_INVALID_PARAMETER; > goto ON_EXIT; > } > > IsAddressOk = FALSE; > > @@ -1041,10 +1051,11 @@ IfConfigSetInterfaceInfo ( > IfCb->IfCfg, > Ip4Config2DataTypeManualAddress, > MappedEvt > ); > if (EFI_ERROR (Status)) { > + ShellStatus = SHELL_ACCESS_DENIED; > goto ON_EXIT; > } > > DataSize = sizeof (EFI_IP4_CONFIG2_MANUAL_ADDRESS); > > @@ -1069,13 +1080,14 @@ IfConfigSetInterfaceInfo ( > IfCb->IfCfg->UnregisterDataNotify ( > IfCb->IfCfg, > Ip4Config2DataTypeManualAddress, > MappedEvt > ); > - > + > if (EFI_ERROR (Status)) { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN > (STR_IFCONFIG_SET_ADDR_FAILED), gShellNetwork1HiiHandle, Status); > + ShellStatus = SHELL_ACCESS_DENIED; > goto ON_EXIT; > } > > // > // Set gateway. > @@ -1086,10 +1098,15 @@ IfConfigSetInterfaceInfo ( > IfCb->IfCfg, > Ip4Config2DataTypeGateway, > DataSize, > &Gateway > ); > + if (EFI_ERROR (Status)) { > + ShellStatus = SHELL_ACCESS_DENIED; > + goto ON_EXIT; > + } > + > VarArg = VarArg->Next; > > } else if (StrCmp (VarArg->Arg, L"dns") == 0) { > // > // Get DNS addresses. > @@ -1107,10 +1124,11 @@ IfConfigSetInterfaceInfo ( > Tmp = VarArg; > Index = 0; > while (Tmp != NULL) { > Status = NetLibStrToIp4 (Tmp->Arg, Dns + Index); > if (EFI_ERROR(Status)) { > + ShellStatus = SHELL_INVALID_PARAMETER; > goto ON_EXIT; > } > Index ++; > Tmp = Tmp->Next; > } > @@ -1126,70 +1144,76 @@ IfConfigSetInterfaceInfo ( > IfCb->IfCfg, > Ip4Config2DataTypeDnsServer, > DataSize, > Dns > ); > + if (EFI_ERROR (Status)) { > + ShellStatus = SHELL_ACCESS_DENIED; > + goto ON_EXIT; > + } > } > } > > ON_EXIT: > if (Dns != NULL) { > FreePool (Dns); > } > > - return Status; > + return ShellStatus; > > } > > /** > The ifconfig command main process. > > @param[in] Private The pointer of IFCONFIG_PRIVATE_DATA. > > - @retval EFI_SUCCESS ifconfig command processed successfully. > + @retval SHELL_SUCCESS ifconfig command processed successfully. > @retval others The ifconfig command process failed. > > **/ > -EFI_STATUS > +SHELL_STATUS > IfConfig ( > IN IFCONFIG_PRIVATE_DATA *Private > ) > { > EFI_STATUS Status; > + SHELL_STATUS ShellStatus; > + > + ShellStatus = SHELL_SUCCESS; > > // > // Get configure information of all interfaces. > // > Status = IfConfigGetInterfaceInfo ( > Private->IfName, > &Private->IfList > ); > - > if (EFI_ERROR (Status)) { > + ShellStatus = SHELL_NOT_FOUND; > goto ON_EXIT; > } > > switch (Private->OpCode) { > case IfConfigOpList: > - Status = IfConfigShowInterfaceInfo (&Private->IfList); > + ShellStatus = IfConfigShowInterfaceInfo (&Private->IfList); > break; > > case IfConfigOpClear: > - Status = IfConfigClearInterfaceInfo (&Private->IfList); > + ShellStatus = IfConfigClearInterfaceInfo (&Private->IfList); > break; > > case IfConfigOpSet: > - Status = IfConfigSetInterfaceInfo (&Private->IfList, Private->VarArg); > + ShellStatus = IfConfigSetInterfaceInfo (&Private->IfList, > Private->VarArg); > break; > > default: > - Status = EFI_ABORTED; > + ShellStatus = SHELL_UNSUPPORTED; > } > > ON_EXIT: > - > - return Status; > + return ShellStatus; > } > > /** > The ifconfig command cleanup process, free the allocated memory. > > @@ -1265,56 +1289,66 @@ ShellCommandRunIfconfig ( > ) > { > EFI_STATUS Status; > IFCONFIG_PRIVATE_DATA *Private; > LIST_ENTRY *ParamPackage; > + SHELL_STATUS ShellStatus; > CONST CHAR16 *ValueStr; > ARG_LIST *ArgList; > CHAR16 *ProblemParam; > CHAR16 *Str; > - > + > + Status = EFI_INVALID_PARAMETER; > Private = NULL; > + ShellStatus = SHELL_SUCCESS; > > Status = ShellCommandLineParseEx (mIfConfigCheckList, &ParamPackage, > &ProblemParam, TRUE, FALSE); > if (EFI_ERROR (Status)) { > - ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), > gShellNetwork1HiiHandle, L"ifconfig", ProblemParam); > + if (Status == EFI_VOLUME_CORRUPTED && ProblemParam != NULL) { > + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), > gShellNetwork1HiiHandle, L"ifconfig", ProblemParam); > + FreePool(ProblemParam); > + ShellStatus = SHELL_INVALID_PARAMETER; > + } else { > + ASSERT(FALSE); > + } > + > goto ON_EXIT; > } > > // > // To handle unsupported option. > // > if (ShellCommandLineGetFlag (ParamPackage, L"-c")) { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN > (STR_IFCONFIG_UNSUPPORTED_OPTION), gShellNetwork1HiiHandle,L"-c"); > + ShellStatus = SHELL_INVALID_PARAMETER; > goto ON_EXIT; > } > > // > // To handle no option. > // > if (!ShellCommandLineGetFlag (ParamPackage, L"-r") && > !ShellCommandLineGetFlag (ParamPackage, L"-s") && > !ShellCommandLineGetFlag (ParamPackage, L"-l")) { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN > (STR_IFCONFIG_LACK_OPTION), gShellNetwork1HiiHandle); > + ShellStatus = SHELL_INVALID_PARAMETER; > goto ON_EXIT; > } > > // > // To handle conflict options. > // > if (((ShellCommandLineGetFlag (ParamPackage, L"-r")) && > (ShellCommandLineGetFlag (ParamPackage, L"-s"))) || > ((ShellCommandLineGetFlag (ParamPackage, L"-r")) && > (ShellCommandLineGetFlag (ParamPackage, L"-l"))) || > ((ShellCommandLineGetFlag (ParamPackage, L"-s")) && > (ShellCommandLineGetFlag (ParamPackage, L"-l")))) { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_CON), > gShellNetwork1HiiHandle, L"ifconfig"); > + ShellStatus = SHELL_INVALID_PARAMETER; > goto ON_EXIT; > } > > - Status = EFI_INVALID_PARAMETER; > - > Private = AllocateZeroPool (sizeof (IFCONFIG_PRIVATE_DATA)); > - > if (Private == NULL) { > - Status = EFI_OUT_OF_RESOURCES; > + ShellStatus = SHELL_OUT_OF_RESOURCES; > goto ON_EXIT; > } > > InitializeListHead (&Private->IfList); > > @@ -1349,10 +1383,11 @@ ShellCommandRunIfconfig ( > // > if (ShellCommandLineGetFlag (ParamPackage, L"-s")) { > ValueStr = ShellCommandLineGetValue (ParamPackage, L"-s"); > if (ValueStr == NULL) { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN > (STR_IFCONFIG_LACK_INTERFACE), gShellNetwork1HiiHandle); > + ShellStatus = SHELL_INVALID_PARAMETER; > goto ON_EXIT; > } > > // > // To split the configuration into multi-section. > @@ -1365,24 +1400,25 @@ ShellCommandRunIfconfig ( > > Private->VarArg = ArgList->Next; > > if (Private->IfName == NULL || Private->VarArg == NULL) { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN > (STR_IFCONFIG_LACK_COMMAND), gShellNetwork1HiiHandle); > + ShellStatus = SHELL_INVALID_PARAMETER; > goto ON_EXIT; > } > } > > // > // Main process of ifconfig. > // > - Status = IfConfig (Private); > + ShellStatus = IfConfig (Private); > > ON_EXIT: > > ShellCommandLineFreeVarList (ParamPackage); > > if (Private != NULL) { > IfConfigCleanup (Private); > } > > - return Status; > + return ShellStatus; > } > -- > 1.9.5.msysgit.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
