diff mbox

pci: aspm: free memory from lists

Message ID 1337108572-4455-1-git-send-email-colin.king@canonical.com
State Accepted
Headers show

Commit Message

Colin Ian King May 15, 2012, 7:02 p.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

free lspci text lists and pci device lists correctly. valgrind
found a bunch of memory allocations that were not being freed.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/pci/aspm/aspm.c |   32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

Comments

Alex Hung May 16, 2012, 2:46 a.m. UTC | #1 
On 05/16/2012 03:02 AM, Colin King wrote:
> From: Colin Ian King<colin.king@canonical.com>
>
> free lspci text lists and pci device lists correctly. valgrind
> found a bunch of memory allocations that were not being freed.
>
> Signed-off-by: Colin Ian King<colin.king@canonical.com>
> ---
>   src/pci/aspm/aspm.c |   32 ++++++++++++++++++++++----------
>   1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c
> index c461ce8..fe6cb60 100644
> --- a/src/pci/aspm/aspm.c
> +++ b/src/pci/aspm/aspm.c
> @@ -188,6 +188,17 @@ int pcie_compare_rp_dev_aspm_registers(fwts_framework *fw,
>   	return ret;
>   }
>
> +static void pci_device_list_free(struct pci_device *head)
> +{
> +	struct pci_device *next;
> +
> +	while (head) {
> +		next = head->next;
> +		free(head);
> +		head = next;
> +	}
> +}
> +
>   int pcie_check_aspm_registers(fwts_framework *fw,
>   				   const fwts_log_level level)
>   {
> @@ -212,8 +223,11 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>   		char *pEnd;
>
>   		device = (struct pci_device *) calloc(1, sizeof(*device));
> -		if (device == NULL)
> +		if (device == NULL) {
> +			pci_device_list_free(head);
> +			fwts_text_list_free(lspci_output);
>   			return FWTS_ERROR;
> +		}
>
>   		device->bus = strtol(line,&pEnd, 16);
>   		device->dev = strtol(pEnd + 1,&pEnd, 16);
> @@ -226,6 +240,7 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>
>   		cur = device;
>   	}
> +	fwts_text_list_free(lspci_output);
>
>   	for (cur = head; cur; cur = cur->next) {
>   		int reg_loc = 0, reg_val = 0;
> @@ -235,10 +250,13 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>   			fw->lspci, cur->bus, cur->dev, cur->func);
>   		if (fwts_pipe_exec(command,&lspci_output) == FWTS_EXEC_ERROR) {
>   			fwts_log_warning(fw, "Could not execute %s", command);
> +			pci_device_list_free(head);
>   			return FWTS_ERROR;
>   		}
> -		if (lspci_output == NULL)
> +		if (lspci_output == NULL) {
> +			pci_device_list_free(head);
>   			return FWTS_ERROR;
> +		}
>
>   		fwts_list_foreach(item, lspci_output) {
>   			char *line = fwts_text_list_text(item);
> @@ -253,6 +271,7 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>   				}
>   			}
>   		}
> +		fwts_text_list_free(lspci_output);
>   	}
>
>   	/* Check aspm registers from the list of pci devices */
> @@ -275,14 +294,7 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>   		}
>   	}
>
> -	cur = head;
> -	while (cur != NULL) {
> -		device = cur->next;
> -		free(cur);
> -		cur = device;
> -	}
> -
> -	fwts_text_list_free(lspci_output);
> +	pci_device_list_free(head);
>
>   	return ret;
>   }

Acked-by: Alex Hung <alex.hung@canonical.com>
Keng-Yu Lin May 17, 2012, 6:12 a.m. UTC | #2 
On Wed, May 16, 2012 at 10:46 AM, Alex Hung <alex.hung@canonical.com> wrote:
> On 05/16/2012 03:02 AM, Colin King wrote:
>>
>> From: Colin Ian King<colin.king@canonical.com>
>>
>> free lspci text lists and pci device lists correctly. valgrind
>> found a bunch of memory allocations that were not being freed.
>>
>> Signed-off-by: Colin Ian King<colin.king@canonical.com>
>> ---
>>  src/pci/aspm/aspm.c |   32 ++++++++++++++++++++++----------
>>  1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c
>> index c461ce8..fe6cb60 100644
>> --- a/src/pci/aspm/aspm.c
>> +++ b/src/pci/aspm/aspm.c
>> @@ -188,6 +188,17 @@ int pcie_compare_rp_dev_aspm_registers(fwts_framework
>> *fw,
>>        return ret;
>>  }
>>
>> +static void pci_device_list_free(struct pci_device *head)
>> +{
>> +       struct pci_device *next;
>> +
>> +       while (head) {
>> +               next = head->next;
>> +               free(head);
>> +               head = next;
>> +       }
>> +}
>> +
>>  int pcie_check_aspm_registers(fwts_framework *fw,
>>                                   const fwts_log_level level)
>>  {
>> @@ -212,8 +223,11 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>>                char *pEnd;
>>
>>                device = (struct pci_device *) calloc(1, sizeof(*device));
>> -               if (device == NULL)
>> +               if (device == NULL) {
>> +                       pci_device_list_free(head);
>> +                       fwts_text_list_free(lspci_output);
>>                        return FWTS_ERROR;
>> +               }
>>
>>                device->bus = strtol(line,&pEnd, 16);
>>                device->dev = strtol(pEnd + 1,&pEnd, 16);
>>
>> @@ -226,6 +240,7 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>>
>>                cur = device;
>>        }
>> +       fwts_text_list_free(lspci_output);
>>
>>        for (cur = head; cur; cur = cur->next) {
>>                int reg_loc = 0, reg_val = 0;
>> @@ -235,10 +250,13 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>>                        fw->lspci, cur->bus, cur->dev, cur->func);
>>                if (fwts_pipe_exec(command,&lspci_output) ==
>> FWTS_EXEC_ERROR) {
>>
>>                        fwts_log_warning(fw, "Could not execute %s",
>> command);
>> +                       pci_device_list_free(head);
>>                        return FWTS_ERROR;
>>                }
>> -               if (lspci_output == NULL)
>> +               if (lspci_output == NULL) {
>> +                       pci_device_list_free(head);
>>                        return FWTS_ERROR;
>> +               }
>>
>>                fwts_list_foreach(item, lspci_output) {
>>                        char *line = fwts_text_list_text(item);
>> @@ -253,6 +271,7 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>>                                }
>>                        }
>>                }
>> +               fwts_text_list_free(lspci_output);
>>        }
>>
>>        /* Check aspm registers from the list of pci devices */
>> @@ -275,14 +294,7 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>>                }
>>        }
>>
>> -       cur = head;
>> -       while (cur != NULL) {
>> -               device = cur->next;
>> -               free(cur);
>> -               cur = device;
>> -       }
>> -
>> -       fwts_text_list_free(lspci_output);
>> +       pci_device_list_free(head);
>>
>>        return ret;
>>  }
>
>
> Acked-by: Alex Hung <alex.hung@canonical.com>
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Ivan Hu May 17, 2012, 10:13 a.m. UTC | #3 
On 05/17/2012 02:12 PM, Keng-Yu Lin wrote:
> On Wed, May 16, 2012 at 10:46 AM, Alex Hung<alex.hung@canonical.com>  wrote:
>> On 05/16/2012 03:02 AM, Colin King wrote:
>>> From: Colin Ian King<colin.king@canonical.com>
>>>
>>> free lspci text lists and pci device lists correctly. valgrind
>>> found a bunch of memory allocations that were not being freed.
>>>
>>> Signed-off-by: Colin Ian King<colin.king@canonical.com>
>>> ---
>>>   src/pci/aspm/aspm.c |   32 ++++++++++++++++++++++----------
>>>   1 file changed, 22 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c
>>> index c461ce8..fe6cb60 100644
>>> --- a/src/pci/aspm/aspm.c
>>> +++ b/src/pci/aspm/aspm.c
>>> @@ -188,6 +188,17 @@ int pcie_compare_rp_dev_aspm_registers(fwts_framework
>>> *fw,
>>>         return ret;
>>>   }
>>>
>>> +static void pci_device_list_free(struct pci_device *head)
>>> +{
>>> +       struct pci_device *next;
>>> +
>>> +       while (head) {
>>> +               next = head->next;
>>> +               free(head);
>>> +               head = next;
>>> +       }
>>> +}
>>> +
>>>   int pcie_check_aspm_registers(fwts_framework *fw,
>>>                                    const fwts_log_level level)
>>>   {
>>> @@ -212,8 +223,11 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>>>                 char *pEnd;
>>>
>>>                 device = (struct pci_device *) calloc(1, sizeof(*device));
>>> -               if (device == NULL)
>>> +               if (device == NULL) {
>>> +                       pci_device_list_free(head);
>>> +                       fwts_text_list_free(lspci_output);
>>>                         return FWTS_ERROR;
>>> +               }
>>>
>>>                 device->bus = strtol(line,&pEnd, 16);
>>>                 device->dev = strtol(pEnd + 1,&pEnd, 16);
>>>
>>> @@ -226,6 +240,7 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>>>
>>>                 cur = device;
>>>         }
>>> +       fwts_text_list_free(lspci_output);
>>>
>>>         for (cur = head; cur; cur = cur->next) {
>>>                 int reg_loc = 0, reg_val = 0;
>>> @@ -235,10 +250,13 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>>>                         fw->lspci, cur->bus, cur->dev, cur->func);
>>>                 if (fwts_pipe_exec(command,&lspci_output) ==
>>> FWTS_EXEC_ERROR) {
>>>
>>>                         fwts_log_warning(fw, "Could not execute %s",
>>> command);
>>> +                       pci_device_list_free(head);
>>>                         return FWTS_ERROR;
>>>                 }
>>> -               if (lspci_output == NULL)
>>> +               if (lspci_output == NULL) {
>>> +                       pci_device_list_free(head);
>>>                         return FWTS_ERROR;
>>> +               }
>>>
>>>                 fwts_list_foreach(item, lspci_output) {
>>>                         char *line = fwts_text_list_text(item);
>>> @@ -253,6 +271,7 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>>>                                 }
>>>                         }
>>>                 }
>>> +               fwts_text_list_free(lspci_output);
>>>         }
>>>
>>>         /* Check aspm registers from the list of pci devices */
>>> @@ -275,14 +294,7 @@ int pcie_check_aspm_registers(fwts_framework *fw,
>>>                 }
>>>         }
>>>
>>> -       cur = head;
>>> -       while (cur != NULL) {
>>> -               device = cur->next;
>>> -               free(cur);
>>> -               cur = device;
>>> -       }
>>> -
>>> -       fwts_text_list_free(lspci_output);
>>> +       pci_device_list_free(head);
>>>
>>>         return ret;
>>>   }
>>
>> Acked-by: Alex Hung<alex.hung@canonical.com>
>>
> Acked-by: Keng-Yu Lin<kengyu@canonical.com>
Acked-by: Ivan Hu<ivan.hu@canonical.com>
diff mbox

Patch

diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c
index c461ce8..fe6cb60 100644
--- a/src/pci/aspm/aspm.c
+++ b/src/pci/aspm/aspm.c
@@ -188,6 +188,17 @@  int pcie_compare_rp_dev_aspm_registers(fwts_framework *fw,
 	return ret;
 }
 
+static void pci_device_list_free(struct pci_device *head)
+{
+	struct pci_device *next;
+
+	while (head) {
+		next = head->next;
+		free(head);
+		head = next;
+	}
+}
+
 int pcie_check_aspm_registers(fwts_framework *fw,
 				   const fwts_log_level level)
 {
@@ -212,8 +223,11 @@  int pcie_check_aspm_registers(fwts_framework *fw,
 		char *pEnd;
 
 		device = (struct pci_device *) calloc(1, sizeof(*device));
-		if (device == NULL)
+		if (device == NULL) {
+			pci_device_list_free(head);
+			fwts_text_list_free(lspci_output);
 			return FWTS_ERROR;
+		}
 
 		device->bus = strtol(line, &pEnd, 16);
 		device->dev = strtol(pEnd + 1, &pEnd, 16);
@@ -226,6 +240,7 @@  int pcie_check_aspm_registers(fwts_framework *fw,
 
 		cur = device;
 	}
+	fwts_text_list_free(lspci_output);
 
 	for (cur = head; cur; cur = cur->next) {
 		int reg_loc = 0, reg_val = 0;
@@ -235,10 +250,13 @@  int pcie_check_aspm_registers(fwts_framework *fw,
 			fw->lspci, cur->bus, cur->dev, cur->func);
 		if (fwts_pipe_exec(command, &lspci_output) == FWTS_EXEC_ERROR) {
 			fwts_log_warning(fw, "Could not execute %s", command);
+			pci_device_list_free(head);
 			return FWTS_ERROR;
 		}
-		if (lspci_output == NULL)
+		if (lspci_output == NULL) {
+			pci_device_list_free(head);
 			return FWTS_ERROR;
+		}
 
 		fwts_list_foreach(item, lspci_output) {
 			char *line = fwts_text_list_text(item);
@@ -253,6 +271,7 @@  int pcie_check_aspm_registers(fwts_framework *fw,
 				}
 			}
 		}
+		fwts_text_list_free(lspci_output);
 	}
 
 	/* Check aspm registers from the list of pci devices */
@@ -275,14 +294,7 @@  int pcie_check_aspm_registers(fwts_framework *fw,
 		}
 	}
 
-	cur = head;
-	while (cur != NULL) {
-		device = cur->next;
-		free(cur);
-		cur = device;
-	}
-
-	fwts_text_list_free(lspci_output);
+	pci_device_list_free(head);
 
 	return ret;
 }