From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 01726AC0E78 for ; Sat, 22 Jul 2023 12:09:54 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=o33qgNWr4bSE6NRQ9i/0heZ+qpgWiGjkgJurV04H9YA=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:X-Received:X-Gm-Message-State:X-Google-Smtp-Source:X-Received:MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20140610; t=1690027793; v=1; b=arJRrpCBurW9nMJMTVnn7h7v0cG23Dkq0zVZlLODYvi4BPtLou6S9hr7PFjNAf2TzuA3gwwi PC1Y4lqdemND32VzYWExUBX/q1zeQG6jCsFRaWsq9mO0y4f3d+fnI5YYBLFJsiRf2j2tTMPVYeq xjKuXC/GZHEplQE0QPu7aXn4= X-Received: by 127.0.0.2 with SMTP id 27q4YY7687511xHV2QdwVbO6; Sat, 22 Jul 2023 05:09:53 -0700 X-Received: from mail-yb1-f169.google.com (mail-yb1-f169.google.com [209.85.219.169]) by mx.groups.io with SMTP id smtpd.web11.4024.1690027792777673946 for ; Sat, 22 Jul 2023 05:09:52 -0700 X-Received: by mail-yb1-f169.google.com with SMTP id 3f1490d57ef6-cf284f4d7afso2802046276.3 for ; Sat, 22 Jul 2023 05:09:52 -0700 (PDT) X-Gm-Message-State: D9jQkAuCPmMSKGYRLImJSySwx7686176AA= X-Google-Smtp-Source: APBJJlHr4jzl3+QHVbCg9ZbnMe1/44fhf+wMbx9iqA13LRLAKbEXPEB2Sec0jrZ850fn2fLrbWo8y058uQ/Hsn94ptg= X-Received: by 2002:a25:2e45:0:b0:d0a:353b:b939 with SMTP id b5-20020a252e45000000b00d0a353bb939mr453284ybn.52.1690027791868; Sat, 22 Jul 2023 05:09:51 -0700 (PDT) MIME-Version: 1.0 References: <20230722081826.301-1-nicklew@nvidia.com> In-Reply-To: <20230722081826.301-1-nicklew@nvidia.com> From: "Mike Maslenkin" Date: Sat, 22 Jul 2023 15:09:15 +0300 Message-ID: Subject: Re: [edk2-devel] [PATCH] RedfishPkg/RedfishRestExDxe: return HTTP status code to caller. To: devel@edk2.groups.io, nicklew@nvidia.com Cc: Abner Chang , Igor Kulchytskyy , Nick Ramirez Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mike.maslenkin@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=arJRrpCB; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Sat, Jul 22, 2023 at 11:18=E2=80=AFAM Nickle Wang via groups.io wrote: > > Return unsupported HTTP status code to caller so caller can handle > HTTP error status code. Current implementation only return EFI error > to caller. Without knowing the HTTP status code, caller has trouble > to handle HTTP request failure. > > Signed-off-by: Nickle Wang > Cc: Abner Chang > Cc: Igor Kulchytskyy > Cc: Nick Ramirez > --- > .../RedfishRestExDxe/RedfishRestExProtocol.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c b/Redfis= hPkg/RedfishRestExDxe/RedfishRestExProtocol.c > index 90973619f2bc..f11cee8542fb 100644 > --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c > +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c > @@ -320,6 +320,18 @@ ReSendRequest:; > DEBUG ((DEBUG_ERROR, "This HTTP Status is not handled!\n")); > DumpHttpStatusCode (DEBUG_REDFISH_NETWORK, ResponseData->Response.St= atusCode); > Status =3D EFI_UNSUPPORTED; > + > + // > + // Deliver status code back to caller so caller can handle it. > + // > + ResponseMessage->Data.Response =3D AllocateZeroPool (sizeof (EFI_HTT= P_RESPONSE_DATA)); > + if (ResponseMessage->Data.Response =3D=3D NULL) { > + Status =3D EFI_OUT_OF_RESOURCES; > + goto ON_EXIT; > + } > + > + ResponseMessage->Data.Response->StatusCode =3D ResponseData->Respons= e.StatusCode; > + > goto ON_EXIT; > } > > @@ -444,11 +456,6 @@ ON_EXIT: > } > > if (EFI_ERROR (Status)) { > - if (ResponseMessage->Data.Response !=3D NULL) { > - FreePool (ResponseMessage->Data.Response); > - ResponseMessage->Data.Response =3D NULL; > - } > - This doesn't make code cleaner. What about resource deallocation after AllocateZeroPool (sizeof (EFI_HTTP_RESPONSE_DATA)) for the common case? I guess this resource deallocation was added for this and there are a lot of 'goto ON_EXIT;' after original AllocateZeroPool call. I would suggest to remove this `if (EFI_ERROR (Status)) {` block at all. As I can see, the only callers of this function are RedfishPkg//PrivateLibrary/RedfishLib/edk2libredfish/src/service.c and edk2-redfish-client//RedfishClientPkg/PrivateLibrary/RedfishLib/edk2libredf= ish/src/service.c. And both use the same pattern of an explicit response data deallocation (RestConfigFreeHttpMessage() call) in case of error status returned. I.e. caller is responsible for the response data deallocation as it owns it= . -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107142): https://edk2.groups.io/g/devel/message/107142 Mute This Topic: https://groups.io/mt/100292372/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-