Re: [Devel] [PATCH] ACPICA / Interpreter: Remove redundant newline
by Zheng, Lv
Hi,
> From: Joe Perches [mailto:[email protected]]
> 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)
5 years, 10 months
Re: [Devel] [PATCH] ACPICA / Interpreter: Remove redundant newline
by Zheng, Lv
The newline is intentional for acpiexec.
So you should fix this issue in acpiexec, aka, in ACPICA upstream.
Thanks
Lv
> From: linux-acpi-owner(a)vger.kernel.org [mailto:[email protected]] On Behalf Of Borislav
> Petkov
> Subject: [PATCH] ACPICA / Interpreter: Remove redundant newline
>
> From: Borislav Petkov <bp(a)suse.de>
>
> acpi_info() already issues a '\n' so remove it in the call.
>
> Signed-off-by: Borislav Petkov <bp(a)suse.de>
> Cc: Robert Moore <robert.moore(a)intel.com>
> Cc: Lv Zheng <lv.zheng(a)intel.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki(a)intel.com>
> Cc: Len Brown <lenb(a)kernel.org>
> Cc: linux-acpi(a)vger.kernel.org
> Cc: devel(a)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(a)vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
5 years, 10 months
[PATCH] Debugger: free Pathname on error return path to fix memory leak
by Colin King
From: Colin Ian King <colin.king(a)canonical.com>
On the error return path when AcpiGetObjectInfo fails the allocated
pathname is not free'd leading to a memory leak. Free pathname
to fix this.
Signed-off-by: Colin Ian King <colin.king(a)canonical.com>
---
source/components/debugger/dbmethod.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/source/components/debugger/dbmethod.c b/source/components/debugger/dbmethod.c
index 5756ecb..22a66d1 100644
--- a/source/components/debugger/dbmethod.c
+++ b/source/components/debugger/dbmethod.c
@@ -551,6 +551,7 @@ AcpiDbWalkForExecute (
Status = AcpiGetObjectInfo (ObjHandle, &ObjInfo);
if (ACPI_FAILURE (Status))
{
+ ACPI_FREE(Pathname);
return (Status);
}
--
2.9.3
5 years, 10 months
Re: [Devel] [PATCH] ACPICA / Interpreter: Remove redundant newline
by Moore, Robert
> Please do not top-post.
You're not going to get a lot of help by scolding me.
Bob
> -----Original Message-----
> From: Borislav Petkov [mailto:[email protected]]
> Sent: Friday, September 9, 2016 11:41 AM
> To: Moore, Robert <robert.moore(a)intel.com>
> Cc: LKML <linux-kernel(a)vger.kernel.org>; Zheng, Lv <lv.zheng(a)intel.com>;
> Wysocki, Rafael J <rafael.j.wysocki(a)intel.com>; Len Brown
> <lenb(a)kernel.org>; linux-acpi(a)vger.kernel.org; devel(a)acpica.org
> Subject: Re: [PATCH] ACPICA / Interpreter: Remove redundant newline
>
> 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).
>
> --
> Regards/Gruss,
> Boris.
>
> ECO tip #101: Trim your mails when you reply.
5 years, 11 months
[PATCH] acpisrc: trivial fix to misaligned code indentation
by Colin King
From: Colin Ian King <colin.king(a)canonical.com>
Insert missing space to correct indentation
Signed-off-by: Colin Ian King <colin.king(a)canonical.com>
---
source/tools/acpisrc/asfile.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source/tools/acpisrc/asfile.c b/source/tools/acpisrc/asfile.c
index 51d1c69..07830c0 100644
--- a/source/tools/acpisrc/asfile.c
+++ b/source/tools/acpisrc/asfile.c
@@ -393,7 +393,7 @@ AsConvertFile (
ConditionalTable = ConversionTable->SourceConditionalTable;
StructTable = ConversionTable->SourceStructTable;
SpecialMacroTable = ConversionTable->SourceSpecialMacroTable;
- break;
+ break;
case FILE_TYPE_HEADER:
--
2.9.3
5 years, 11 months