Patchwork acpi: acpidump: use size_t instead of int

login
register
mail settings
Submitter Colin King
Date April 10, 2012, 2:25 p.m.
Message ID <1334067909-1644-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/151581/
State Rejected
Headers show

Comments

Colin King - April 10, 2012, 2:25 p.m.
From: Colin Ian King <colin.king@canonical.com>

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpi/acpidump/acpidump.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Alex Hung - April 11, 2012, 1:45 a.m.
On 04/10/2012 10:25 PM, Colin King wrote:
> From: Colin Ian King<colin.king@canonical.com>
>
> Signed-off-by: Colin Ian King<colin.king@canonical.com>
> ---
>   src/acpi/acpidump/acpidump.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/acpi/acpidump/acpidump.c b/src/acpi/acpidump/acpidump.c
> index 6b20ca7..c673768 100644
> --- a/src/acpi/acpidump/acpidump.c
> +++ b/src/acpi/acpidump/acpidump.c
> @@ -665,7 +665,7 @@ static void acpidump_xsdt(fwts_framework *fw, uint8_t *data, int length)
>   static void acpidump_madt(fwts_framework *fw, uint8_t *data, int length)
>   {
>   	int i = 0;
> -	int n;
> +	size_t n;
>   	int offset = 0;
>
>   	fwts_acpidump_field fields[] = {

The variable n seems only used once:
                                 n = strlen(local_sapic->uid_string) + 1;
                                 skip = 
(sizeof(fwts_acpi_madt_local_sapic) + n);

Will it be better to remove the variable and change it to
                                  skip = 
(sizeof(fwts_acpi_madt_local_sapic) + strlen(local_sapic->uid_string) + 1);
and then we don't need to worry about its type?
Colin King - April 11, 2012, 7:56 a.m.
On 11/04/12 02:45, Alex Hung wrote:
> On 04/10/2012 10:25 PM, Colin King wrote:
>> From: Colin Ian King<colin.king@canonical.com>
>>
>> Signed-off-by: Colin Ian King<colin.king@canonical.com>
>> ---
>> src/acpi/acpidump/acpidump.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/acpi/acpidump/acpidump.c b/src/acpi/acpidump/acpidump.c
>> index 6b20ca7..c673768 100644
>> --- a/src/acpi/acpidump/acpidump.c
>> +++ b/src/acpi/acpidump/acpidump.c
>> @@ -665,7 +665,7 @@ static void acpidump_xsdt(fwts_framework *fw,
>> uint8_t *data, int length)
>> static void acpidump_madt(fwts_framework *fw, uint8_t *data, int length)
>> {
>> int i = 0;
>> - int n;
>> + size_t n;
>> int offset = 0;
>>
>> fwts_acpidump_field fields[] = {
>
> The variable n seems only used once:
> n = strlen(local_sapic->uid_string) + 1;
> skip = (sizeof(fwts_acpi_madt_local_sapic) + n);
>
> Will it be better to remove the variable and change it to
> skip = (sizeof(fwts_acpi_madt_local_sapic) +
> strlen(local_sapic->uid_string) + 1);
> and then we don't need to worry about its type?
>
I'm happy with that suggestion - I will tidy that up and re-submit once 
I've done my next round of changes.

Patch

diff --git a/src/acpi/acpidump/acpidump.c b/src/acpi/acpidump/acpidump.c
index 6b20ca7..c673768 100644
--- a/src/acpi/acpidump/acpidump.c
+++ b/src/acpi/acpidump/acpidump.c
@@ -665,7 +665,7 @@  static void acpidump_xsdt(fwts_framework *fw, uint8_t *data, int length)
 static void acpidump_madt(fwts_framework *fw, uint8_t *data, int length)
 {
 	int i = 0;
-	int n;
+	size_t n;
 	int offset = 0;
 
 	fwts_acpidump_field fields[] = {