Message ID | 20160909161253.30952-1-bp@alien8.de (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
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
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).
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
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.
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
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
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
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.
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
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 --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",