diff mbox

ACPICA / Interpreter: Remove redundant newline

Message ID 20160909161253.30952-1-bp@alien8.de (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Borislav Petkov Sept. 9, 2016, 4:12 p.m. UTC
From: Borislav Petkov <bp@suse.de>

acpi_info() already issues a '\n' so remove it in the call.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Robert Moore <robert.moore@intel.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
Cc: devel@acpica.org
---
 drivers/acpi/acpica/tbxfload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Moore, Robert Sept. 9, 2016, 6:26 p.m. UTC | #1
Is this a big deal?

We do this on purpose for AcpiExec, to make the screen output more readable.


> -----Original Message-----
> From: Borislav Petkov [mailto:bp@alien8.de]
> Sent: Friday, September 9, 2016 9:13 AM
> To: LKML <linux-kernel@vger.kernel.org>
> Cc: Moore, Robert <robert.moore@intel.com>; Zheng, Lv
> <lv.zheng@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com>;
> Len Brown <lenb@kernel.org>; linux-acpi@vger.kernel.org;
> devel@acpica.org
> Subject: [PATCH] ACPICA / Interpreter: Remove redundant newline
> 
> From: Borislav Petkov <bp@suse.de>
> 
> acpi_info() already issues a '\n' so remove it in the call.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Robert Moore <robert.moore@intel.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> Cc: devel@acpica.org
> ---
>  drivers/acpi/acpica/tbxfload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/acpica/tbxfload.c
> b/drivers/acpi/acpica/tbxfload.c index ac71abcd32bb..e7119b7ccd79 100644
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -240,7 +240,7 @@ acpi_status acpi_tb_load_namespace(void)
>  	}
> 
>  	if (!tables_failed) {
> -		ACPI_INFO(("%u ACPI AML tables successfully acquired and
> loaded\n", tables_loaded));
> +		ACPI_INFO(("%u ACPI AML tables successfully acquired and
> loaded",
> +tables_loaded));
>  	} else {
>  		ACPI_ERROR((AE_INFO,
>  			    "%u table load failures, %u successful",
> --
> 2.10.0

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov Sept. 9, 2016, 6:40 p.m. UTC | #2
On Fri, Sep 09, 2016 at 06:26:17PM +0000, Moore, Robert wrote:
> Is this a big deal?
> 
> We do this on purpose for AcpiExec, to make the screen output more readable.

Please do not top-post.

What do you mean "big deal"? All other ACPI_INFO calls don't have a "\n"
at the end except this one. How does one "\n" make some output more
readable?

(Btw, I have no idea what AcpiExec is. Grepping the kernel tree doesn't
give any results).
Moore, Robert Sept. 9, 2016, 6:46 p.m. UTC | #3
DQo+IFBsZWFzZSBkbyBub3QgdG9wLXBvc3QuDQoNCg0KWW91J3JlIG5vdCBnb2luZyB0byBnZXQg
YSBsb3Qgb2YgaGVscCBieSBzY29sZGluZyBtZS4NCkJvYg0KDQoNCg0KDQo+IC0tLS0tT3JpZ2lu
YWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IEJvcmlzbGF2IFBldGtvdiBbbWFpbHRvOmJwQGFsaWVu
OC5kZV0NCj4gU2VudDogRnJpZGF5LCBTZXB0ZW1iZXIgOSwgMjAxNiAxMTo0MSBBTQ0KPiBUbzog
TW9vcmUsIFJvYmVydCA8cm9iZXJ0Lm1vb3JlQGludGVsLmNvbT4NCj4gQ2M6IExLTUwgPGxpbnV4
LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc+OyBaaGVuZywgTHYgPGx2LnpoZW5nQGludGVsLmNvbT47
DQo+IFd5c29ja2ksIFJhZmFlbCBKIDxyYWZhZWwuai53eXNvY2tpQGludGVsLmNvbT47IExlbiBC
cm93bg0KPiA8bGVuYkBrZXJuZWwub3JnPjsgbGludXgtYWNwaUB2Z2VyLmtlcm5lbC5vcmc7IGRl
dmVsQGFjcGljYS5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSF0gQUNQSUNBIC8gSW50ZXJwcmV0
ZXI6IFJlbW92ZSByZWR1bmRhbnQgbmV3bGluZQ0KPiANCj4gT24gRnJpLCBTZXAgMDksIDIwMTYg
YXQgMDY6MjY6MTdQTSArMDAwMCwgTW9vcmUsIFJvYmVydCB3cm90ZToNCj4gPiBJcyB0aGlzIGEg
YmlnIGRlYWw/DQo+ID4NCj4gPiBXZSBkbyB0aGlzIG9uIHB1cnBvc2UgZm9yIEFjcGlFeGVjLCB0
byBtYWtlIHRoZSBzY3JlZW4gb3V0cHV0IG1vcmUNCj4gcmVhZGFibGUuDQo+IA0KPiBQbGVhc2Ug
ZG8gbm90IHRvcC1wb3N0Lg0KPiANCj4gV2hhdCBkbyB5b3UgbWVhbiAiYmlnIGRlYWwiPyBBbGwg
b3RoZXIgQUNQSV9JTkZPIGNhbGxzIGRvbid0IGhhdmUgYSAiXG4iDQo+IGF0IHRoZSBlbmQgZXhj
ZXB0IHRoaXMgb25lLiBIb3cgZG9lcyBvbmUgIlxuIiBtYWtlIHNvbWUgb3V0cHV0IG1vcmUNCj4g
cmVhZGFibGU/DQo+IA0KPiAoQnR3LCBJIGhhdmUgbm8gaWRlYSB3aGF0IEFjcGlFeGVjIGlzLiBH
cmVwcGluZyB0aGUga2VybmVsIHRyZWUgZG9lc24ndA0KPiBnaXZlIGFueSByZXN1bHRzKS4NCj4g
DQo+IC0tDQo+IFJlZ2FyZHMvR3J1c3MsDQo+ICAgICBCb3Jpcy4NCj4gDQo+IEVDTyB0aXAgIzEw
MTogVHJpbSB5b3VyIG1haWxzIHdoZW4geW91IHJlcGx5Lg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov Sept. 9, 2016, 6:50 p.m. UTC | #4
On Fri, Sep 09, 2016 at 06:46:20PM +0000, Moore, Robert wrote:
> 
> > Please do not top-post.
> 
> 
> You're not going to get a lot of help by scolding me.

How is "Please do not top-post" scolding you? The stress being on
"Please".

How much more polite can I be?!?

Geez.
Joe Perches Sept. 9, 2016, 6:55 p.m. UTC | #5
On Fri, 2016-09-09 at 20:40 +0200, Borislav Petkov wrote:
> On Fri, Sep 09, 2016 at 06:26:17PM +0000, Moore, Robert wrote:
> > Is this a big deal?
> > We do this on purpose for AcpiExec, to make the screen output more readable.
[]
> What do you mean "big deal"? All other ACPI_INFO calls don't have a "\n"
> at the end except this one. How does one "\n" make some output more
> readable?

Blank lines in logging/dmesg generally don't add value.

I would prefer if the unnecessary double parentheses also
were removed in these macro uses and ##__VA_ARGS__ was
used instead.

/*
 * Error reporting. Callers module and line number are inserted by AE_INFO,
 * the plist contains a set of parens to allow variable-length lists.
 * These macros are used for both the debug and non-debug versions of the code.
 */
#define ACPI_INFO(plist)                acpi_info plist
#define ACPI_WARNING(plist)             acpi_warning plist
#define ACPI_EXCEPTION(plist)           acpi_exception plist
#define ACPI_ERROR(plist)               acpi_error plist
#define ACPI_BIOS_WARNING(plist)        acpi_bios_warning plist
#define ACPI_BIOS_ERROR(plist)          acpi_bios_error plist

It would also be good if format/argument verification
was done here and in the non-debug macro variants.

#define ACPI_INFO(plist)
#define ACPI_WARNING(plist)
#define ACPI_EXCEPTION(plist)
#define ACPI_ERROR(plist)
#define ACPI_BIOS_WARNING(plist)
#define ACPI_BIOS_ERROR(plist)

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lv Zheng Sept. 14, 2016, 3:09 a.m. UTC | #6
The newline is intentional for acpiexec.
So you should fix this issue in acpiexec, aka, in ACPICA upstream.

Thanks
Lv

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Borislav
> Petkov
> Subject: [PATCH] ACPICA / Interpreter: Remove redundant newline
> 
> From: Borislav Petkov <bp@suse.de>
> 
> acpi_info() already issues a '\n' so remove it in the call.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Robert Moore <robert.moore@intel.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> Cc: devel@acpica.org
> ---
>  drivers/acpi/acpica/tbxfload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> index ac71abcd32bb..e7119b7ccd79 100644
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -240,7 +240,7 @@ acpi_status acpi_tb_load_namespace(void)
>  	}
> 
>  	if (!tables_failed) {
> -		ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded\n", tables_loaded));
> +		ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
>  	} else {
>  		ACPI_ERROR((AE_INFO,
>  			    "%u table load failures, %u successful",
> --
> 2.10.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lv Zheng Sept. 14, 2016, 3:14 a.m. UTC | #7
Hi,

> From: Joe Perches [mailto:joe@perches.com]
> Subject: Re: [PATCH] ACPICA / Interpreter: Remove redundant newline
> 
> On Fri, 2016-09-09 at 20:40 +0200, Borislav Petkov wrote:
> > On Fri, Sep 09, 2016 at 06:26:17PM +0000, Moore, Robert wrote:
> > > Is this a big deal?
> > > We do this on purpose for AcpiExec, to make the screen output more readable.
> []
> > What do you mean "big deal"? All other ACPI_INFO calls don't have a "\n"
> > at the end except this one. How does one "\n" make some output more
> > readable?
> 
> Blank lines in logging/dmesg generally don't add value.
> 
> I would prefer if the unnecessary double parentheses also
> were removed in these macro uses and ##__VA_ARGS__ was
> used instead.

Ideally correct.
But __VA_ARGS__ is not portable.
And ACPICA is used in other environment.
I'd prefer to eliminate debugging/logging macros, but implement debugging/logging functions instead.
As stdarg is more portable than __VA_ARGS__.

Thanks and best regards
Lv

> 
> /*
>  * Error reporting. Callers module and line number are inserted by AE_INFO,
>  * the plist contains a set of parens to allow variable-length lists.
>  * These macros are used for both the debug and non-debug versions of the code.
>  */
> #define ACPI_INFO(plist)                acpi_info plist
> #define ACPI_WARNING(plist)             acpi_warning plist
> #define ACPI_EXCEPTION(plist)           acpi_exception plist
> #define ACPI_ERROR(plist)               acpi_error plist
> #define ACPI_BIOS_WARNING(plist)        acpi_bios_warning plist
> #define ACPI_BIOS_ERROR(plist)          acpi_bios_error plist
> 
> It would also be good if format/argument verification
> was done here and in the non-debug macro variants.
> 
> #define ACPI_INFO(plist)
> #define ACPI_WARNING(plist)
> #define ACPI_EXCEPTION(plist)
> #define ACPI_ERROR(plist)
> #define ACPI_BIOS_WARNING(plist)
> #define ACPI_BIOS_ERROR(plist)

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov Sept. 14, 2016, 11:30 a.m. UTC | #8
On Wed, Sep 14, 2016 at 03:09:25AM +0000, Zheng, Lv wrote:
> The newline is intentional for acpiexec.
> So you should fix this issue in acpiexec, aka, in ACPICA upstream.

If you elaborate as to from where I get the sources and against which
branch/version/...?, I can try to send you a patch.
Lv Zheng Sept. 19, 2016, 9:30 a.m. UTC | #9
SGksDQoNCj4gRnJvbTogbGludXgtYWNwaS1vd25lckB2Z2VyLmtlcm5lbC5vcmcgW21haWx0bzps
aW51eC1hY3BpLW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVoYWxmIE9mIEJvcmlzbGF2DQo+
IFBldGtvdg0KPiBTdWJqZWN0OiBSZTogW1BBVENIXSBBQ1BJQ0EgLyBJbnRlcnByZXRlcjogUmVt
b3ZlIHJlZHVuZGFudCBuZXdsaW5lDQo+IA0KPiBPbiBXZWQsIFNlcCAxNCwgMjAxNiBhdCAwMzow
OToyNUFNICswMDAwLCBaaGVuZywgTHYgd3JvdGU6DQo+ID4gVGhlIG5ld2xpbmUgaXMgaW50ZW50
aW9uYWwgZm9yIGFjcGlleGVjLg0KPiA+IFNvIHlvdSBzaG91bGQgZml4IHRoaXMgaXNzdWUgaW4g
YWNwaWV4ZWMsIGFrYSwgaW4gQUNQSUNBIHVwc3RyZWFtLg0KPiANCj4gSWYgeW91IGVsYWJvcmF0
ZSBhcyB0byBmcm9tIHdoZXJlIEkgZ2V0IHRoZSBzb3VyY2VzIGFuZCBhZ2FpbnN0IHdoaWNoDQo+
IGJyYW5jaC92ZXJzaW9uLy4uLj8sIEkgY2FuIHRyeSB0byBzZW5kIHlvdSBhIHBhdGNoLg0KDQpZ
b3UgY2FuIG9idGFpbiBhIGxvY2FsIGNvcHkgb2YgYWNwaWNhLmdpdCB2aWE6DQokIGdpdCBjbG9u
ZSBodHRwczovL2dpdGh1Yi5jb20vYWNwaWNhL2FjcGljYQ0KDQpUaGUgZml4IHBhdGNoIHNob3Vs
ZCBvbmx5IGJlIHNlbnQgdG8gZGV2ZWxAYWNwaWNhLm9yZy4NCkFuZCB5b3UnbGwgc2VlIGl0IGlu
IExpbnV4IHVwc3RyZWFtIGFmdGVyIGFuIGFjcGljYSByZWxlYXNlIGN5Y2xlLg0KDQpSZWdhcmRz
DQpMdg0KDQo+IA0KPiAtLQ0KPiBSZWdhcmRzL0dydXNzLA0KPiAgICAgQm9yaXMuDQo+IA0KPiBF
Q08gdGlwICMxMDE6IFRyaW0geW91ciBtYWlscyB3aGVuIHlvdSByZXBseS4NCj4gLS0NCj4gLS0N
Cj4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2Ny
aWJlIGxpbnV4LWFjcGkiIGluDQo+IHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9A
dmdlci5rZXJuZWwub3JnDQo+IE1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtl
cm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbA0K
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov Sept. 19, 2016, 10:12 a.m. UTC | #10
On Mon, Sep 19, 2016 at 09:30:44AM +0000, Zheng, Lv wrote:
> You can obtain a local copy of acpica.git via:
> $ git clone https://github.com/acpica/acpica
> 
> The fix patch should only be sent to devel@acpica.org.
> And you'll see it in Linux upstream after an acpica release cycle.

No need, Robert did it already:

1d435008fd9e ("Update an info message during table load phase.")

Thanks.
diff mbox

Patch

diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
index ac71abcd32bb..e7119b7ccd79 100644
--- a/drivers/acpi/acpica/tbxfload.c
+++ b/drivers/acpi/acpica/tbxfload.c
@@ -240,7 +240,7 @@  acpi_status acpi_tb_load_namespace(void)
 	}
 
 	if (!tables_failed) {
-		ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded\n", tables_loaded));
+		ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
 	} else {
 		ACPI_ERROR((AE_INFO,
 			    "%u table load failures, %u successful",