linux-kernel - Re: [PATCH 2/30] ACPI-processor: Improve two size determinations in acpi_processor_get_performance

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 10 Sep 2016 13:47:28 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     SF Markus Elfring <elfring@...rs.sourceforge.net>
Cc:     linux-acpi@...r.kernel.org, Len Brown <lenb@...nel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org,
        Julia Lawall <julia.lawall@...6.fr>,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH 2/30] ACPI-processor: Improve two size determinations in
 acpi_processor_get_performance_states()

On Sat, Sep 10, 2016 at 11:36:41AM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@...rs.sourceforge.net>
> Date: Tue, 6 Sep 2016 11:11:12 +0200
> 
> Replace the specification of a data structure by pointer dereferences
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring <elfring@...rs.sourceforge.net>
> ---
>  drivers/acpi/processor_perflib.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index 78f9025..004e24c 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -358,7 +358,7 @@ static int acpi_processor_get_performance_states(struct acpi_processor *pr)
>  
>  		struct acpi_processor_px *px = &(pr->performance->states[i]);
>  
> -		state.length = sizeof(struct acpi_processor_px);
> +		state.length = sizeof(*px);

How is this better?

Now the person reading the code has to go and lookup what px is in order
to know what its size is and which goes in there. Sure, it is a couple
of lines up but this is not always the case...

>  		state.pointer = px;
>  
>  		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Extracting state %d\n", i));
> @@ -400,7 +400,8 @@ static int acpi_processor_get_performance_states(struct acpi_processor *pr)
>  				 * Copy this valid entry over last_invalid entry
>  				 */
>  				memcpy(&(pr->performance->states[last_invalid]),
> -				       px, sizeof(struct acpi_processor_px));
> +				       px,
> +				       sizeof(*px));
>  				++last_invalid;

... like here, for example.

In any case, I won't take this one if I were the maintainer.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.