Hi Mike, How about we add code to release “PendingSettingUri” at the end of RedfishResourceConsumeResource(). I think this would help to address memory leak issue. And we keep “Private->Uri” to be just the reference to “Uri” from caller and we don’t release “Private->Uri”. Caller always has responsibility to release memory it allocated. Then, I like to modify below code to match my logic above: // // Initialize collection path // CollectionUri = RedfishGetUri (ResourceUri); if (CollectionUri == NULL) { ASSERT (FALSE); FreePool (ResourceUri); return EFI_OUT_OF_RESOURCES; } Status = HandleResource (Private, CollectionUri); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "%a, process external resource: %a failed: %r\n", __func__, CollectionUri, Status)); } FreePool (CollectionUri); In this way, we don’ have huge changes to fix memory leak issue as you mentioned. (removing Private->Uri) Please let me know if this makes sense to you or not. Thanks, Nickle From: M M Sent: Friday, January 26, 2024 11:41 PM To: Chang, Abner Cc: devel@edk2.groups.io; Nickle Wang ; Igor Kulchytskyy Subject: Re: [edk2-devel] [edk2-redfish-client][PATCH V2] RedfishClientPkg/FeatureDriver: ComputerSystem_1_13_0 driver External email: Use caution opening links or attachments BTW did you consider to remove this Private->Uri someday at all ? I tried to remove it, but since it declared in a common header for all redfish client feature drivers those changes was huge comparing to a small memory leak I tried to fix. And now such pattern spreads into new feature drivers. In short: there is no need to cache this value in Private->Uri, but pass it as an argument down to stack in EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL implementation. I think it just small design flaw. The code I'm talking about: + // + // Check and see if "@Redfish.Settings" exist or not. + // + ZeroMem (&PendingSettingResponse, sizeof (REDFISH_RESPONSE)); + Status = GetPendingSettings ( + Private->RedfishService, + Response.Payload, + &PendingSettingResponse, + &PendingSettingUri + ); + if (!EFI_ERROR (Status)) { + DEBUG ((REDFISH_DEBUG_TRACE, "%a: @Redfish.Settings found: %s\n", __func__, PendingSettingUri)); + Private->Uri = PendingSettingUri; + ExpectedResponse = &PendingSettingResponse; + } else { + DEBUG ((REDFISH_DEBUG_TRACE, "%a: No @Redfish.Settings is found\n", __func__)); + Private->Uri = Uri; + ExpectedResponse = &Response; + } In this pattern PendingSettingUri is leaked, since Private->Uri belongs to caller and never released. But... in fact it is not true for Feature/Bios driver, and actually leaked resource is original Private->Uri string. This is because these EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL function called by HandleResource() function, which in turns called from here: // // Initialize collection path // Private->Uri = RedfishGetUri (ResourceUri); if (Private->Uri == NULL) { ASSERT (FALSE); FreePool (ResourceUri); return EFI_OUT_OF_RESOURCES; } Status = HandleResource (Private, Private->Uri); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "%a, process external resource: %a failed: %r\n", __func__, Private->Uri, Status)); } FreePool (Private->Uri); Too many "Private->Uri" with a different value. PS: Funny, just pay attention to the log trace above - %a used for Private->Uri. Probably it was one of the factors induced me to write previous message :) Regards, Mike. On 26. 1. 2024., at 17:13, Chang, Abner > wrote: [AMD Official Use Only - General] It is no problem. 😊 Thank you Mike for looking into this patch. Abner -----Original Message----- From: M M > Sent: Friday, January 26, 2024 10:11 PM To: Chang, Abner > Cc: devel@edk2.groups.io; Nickle Wang >; Igor Kulchytskyy > Subject: Re: [edk2-devel] [edk2-redfish-client][PATCH V2] RedfishClientPkg/FeatureDriver: ComputerSystem_1_13_0 driver Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. You are right. Sorry for the noise! No problems here. I mixed up with tags in my editor while looked into patch. Regards, Mike On 26. 1. 2024., at 17:03, Chang, Abner > wrote: [AMD Official Use Only - General] Hi Mike, I can't identify the issue on %s as Private->Uri is defined as EFI_STRING, or I miss something? Thanks Abner -----Original Message----- From: M M > Sent: Friday, January 26, 2024 9:55 PM To: devel@edk2.groups.io; Nickle Wang > Cc: Chang, Abner >; Igor Kulchytskyy > Subject: Re: [edk2-devel] [edk2-redfish-client][PATCH V2] RedfishClientPkg/FeatureDriver: ComputerSystem_1_13_0 driver Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. Hi Abner, On 26. 1. 2024., at 05:20, Nickle Wang via groups.io > wrote: Hi Abner, Same minor issue as 1_5_0. Please add "%a:" to below DEBUG call. + DEBUG ((DEBUG_MANAGEABILITY, " No platform Redfish ConfigureLang found for %s\n", __func__, Private->Uri)); Regards, Nickle There are two issues in this string. The second one is incorrect %s format used for Private->Uri. This issue exists in V3 not only in the line mentioned above. Regards, Mike. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114765): https://edk2.groups.io/g/devel/message/114765 Mute This Topic: https://groups.io/mt/103967446/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-