linux-kernel - RE: [PATCH 01/30] ACPICA: Linuxize: reduce divergences for 20160212 release

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]

Message-ID: <1AE640813FDE7649BE1B193DEA596E883BB63FB7@SHSMSX101.ccr.corp.intel.com>
Date:	Thu, 24 Mar 2016 06:19:33 +0000
From:	"Zheng, Lv" <lv.zheng@...el.com>
To:	Joe Perches <joe@...ches.com>,
	"Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	"Brown, Len" <len.brown@...el.com>
CC:	Lv Zheng <zetalog@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: RE: [PATCH 01/30] ACPICA: Linuxize: reduce divergences for 20160212
 release

Hi,

First, thanks for the revew.

> From: Joe Perches [mailto:joe@...ches.com]
> Subject: Re: [PATCH 01/30] ACPICA: Linuxize: reduce divergences for 20160212
> release
> 
> On Thu, 2016-03-24 at 09:38 +0800, Lv Zheng wrote:
> > The patch reduces source code differences between the Linux kernel and the
> > ACPICA upstream so that the linuxized ACPICA 20160212 release can be
> > applied with reduced human intervention.
> 
> In the very first patch fragment:
> 
> > diff --git a/drivers/acpi/acpica/hwregs.c b/drivers/acpi/acpica/hwregs.c
> []
> > @@ -152,7 +152,7 @@ acpi_hw_validate_register(struct
> acpi_generic_address *reg,
> >   *
> >   **************************************************************
> ****************/
> >
> > -acpi_status acpi_hw_read(u32 *value, struct acpi_generic_address *reg)
> > +acpi_status acpi_hw_read(u32 *value, struct acpi_generic_address * reg)
> 
> The second argument * style appears the opposite of normal style
> and a different style than the first argument * style.
[Lv Zheng] 
The file is drivers/acpi/acpica/hwregs.c, which is coming from ACPICA upstream.
So this is a result of "ACPICA release".
In other words, this is a result of a "process".
In order to fix this, things need to be done in "ACPICA release scripts".
Which should be done in https://github.com/acpica/acpica/blob/master/generate/linux/libacpica.sh.
Otherwise, "ACPICA release" process will require human intervention.

So please leave this patch fragment as is.
It will be automatically fixed if the "ACPICA release" process is fixed.
And if you don't leave this fragment as is, the "ACPICA release" process will get hurt.

> 
> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> []
> > @@ -582,7 +582,7 @@ static char
> acpi_os_name[ACPI_MAX_OVERRIDE_LEN];
> >
> >  acpi_status
> >  acpi_os_predefined_override(const struct acpi_predefined_names *init_val,
> > -			    char **new_val)
> > +			    acpi_string *new_val)
> 
> And here:
> 
> acpi_string pointer style 1:
[Lv Zheng] 
This file drivers/acpi/osl.c is not in the ACPICA upstream.
It is Linux specific, so we use Linux coding style.
Thus this is a result of a "human".

> 
> > diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h
> []
> > @@ -96,7 +96,7 @@ acpi_physical_address acpi_os_get_root_pointer(void);
> >  #ifndef ACPI_USE_ALTERNATE_PROTOTYPE_acpi_os_predefined_override
> >  acpi_status
> >  acpi_os_predefined_override(const struct acpi_predefined_names *init_val,
> > -			    char **new_val);
> > +			    acpi_string * new_val);
> 
> acpi_string pointer style 2:
> 
> There are varying styles for acpi_string *
[Lv Zheng] 
This is ACPICA upstream file modification.
Also a result of a "process".

> 
> So far, this just looks sloppy.
[Lv Zheng] 
As I said, please ignore "process" issues.
I need them to be wrong in order not to modify every "process" generated patches manually.
You can find many such kind of indent issues in drivers/acpi/acpica, include/acpi, tools/power/acpi.
They are there for good reasons.
I'll cleanup such issues from "ACPICA release" process.

> Should the rest be reviewed?

[Lv Zheng] 
Yes, please.

Thanks and best regards
-Lv