RE: [RFC PATCH] acpi: Use a more normal logging style for ACPI_<LEVEL> calls
From: Moore, Robert
Date: Tue Mar 29 2016 - 11:27:22 EST
Probably not, because ACPI_INFO is a public (and documented) macro, and ACPICA is used in many operating systems.
The double parens are there to allow for variable-length arguments, I believe.
Bob
> -----Original Message-----
> From: Joe Perches [mailto:joe@xxxxxxxxxxx]
> Sent: Monday, March 28, 2016 8:30 PM
> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len Brown
> Cc: linux-acpi@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx; linux-kernel
> Subject: [RFC PATCH] acpi: Use a more normal logging style for
> ACPI_<LEVEL> calls
>
> This is just an example of a conversion of ACPI_INFO to a more typical
> kernel use style. All of the other ACPI_<LEVEL> calls would also need
> conversion.
>
> Almost all logging functions and macros in the kernel are lower case and
> nearly all use formats with terminating newlines.
>
> ACPI uses upper case macros and no terminating newline in the format.
>
> Some of the uses though _do_ have newlines. This can cause undesired
> newlines in dmesg output.
>
> Also, the ACPI_<LEVEL> macros use a somewhat odd and unpleasant style with
> double parentheses.
>
> Convert this to a lower case macro, add terminating newlines to formats
> and remove the unnecessary extra parentheses.
>
> Rename the logging function to _acpi_info and have the acpi_info macros
> call this _acpi_info function.
>
> Remove the newline from the _acpi_info call.
>
> This means that all calls to acpi_info are complete and it is not possible
> for any other message to be interleaved into this message.
>
> Miscellanea:
>
> o Coalesce formats
> o Realign arguments
> o Coalesce arguments even if > 80 columns
> ---
> drivers/acpi/acpica/dbcmds.c | 4 ++--
> drivers/acpi/acpica/dsmethod.c | 6 ++----
> drivers/acpi/acpica/dsobject.c | 5 ++---
> drivers/acpi/acpica/evgpeblk.c | 8 ++++----
> drivers/acpi/acpica/evgpeinit.c | 2 +-
> drivers/acpi/acpica/exconfig.c | 4 ++--
> drivers/acpi/acpica/nseval.c | 4 ++--
> drivers/acpi/acpica/tbinstal.c | 15 +++++++--------
> drivers/acpi/acpica/tbprint.c | 36 ++++++++++++++++--------------------
> drivers/acpi/acpica/tbutils.c | 3 ++-
> drivers/acpi/acpica/tbxfload.c | 5 +++--
> drivers/acpi/acpica/uttrack.c | 2 +-
> drivers/acpi/acpica/utxferror.c | 7 +++----
> include/acpi/acoutput.h | 6 +++---
> include/acpi/acpixf.h | 2 +-
> 15 files changed, 51 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/acpi/acpica/dbcmds.c b/drivers/acpi/acpica/dbcmds.c
> index 772178c..1ef21be 100644
> --- a/drivers/acpi/acpica/dbcmds.c
> +++ b/drivers/acpi/acpica/dbcmds.c
> @@ -348,8 +348,8 @@ void acpi_db_display_table_info(char *table_arg)
> } else {
> /* If the pointer is null, the table has been unloaded
> */
>
> - ACPI_INFO(("%4.4s - Table has been unloaded",
> - table_desc->signature.ascii));
> + acpi_info("%4.4s - Table has been unloaded\n",
> + table_desc->signature.ascii);
> }
> }
> }
> diff --git a/drivers/acpi/acpica/dsmethod.c
> b/drivers/acpi/acpica/dsmethod.c index 1982310..4730b0a 100644
> --- a/drivers/acpi/acpica/dsmethod.c
> +++ b/drivers/acpi/acpica/dsmethod.c
> @@ -809,10 +809,8 @@ acpi_ds_terminate_control_method(union
> acpi_operand_object *method_desc,
> if (method_desc->method.
> info_flags & ACPI_METHOD_SERIALIZED_PENDING) {
> if (walk_state) {
> - ACPI_INFO(("Marking method %4.4s as Serialized "
> - "because of AE_ALREADY_EXISTS error",
> - walk_state->method_node->name.
> - ascii));
> + acpi_info("Marking method %4.4s as Serialized
> because of AE_ALREADY_EXISTS error\n",
> + walk_state->method_node->name.ascii);
> }
>
> /*
> diff --git a/drivers/acpi/acpica/dsobject.c
> b/drivers/acpi/acpica/dsobject.c index a91de2b..6787274 100644
> --- a/drivers/acpi/acpica/dsobject.c
> +++ b/drivers/acpi/acpica/dsobject.c
> @@ -524,9 +524,8 @@ acpi_ds_build_internal_package_obj(struct
> acpi_walk_state *walk_state,
> arg = arg->common.next;
> }
>
> - ACPI_INFO(("Actual Package length (%u) is larger than "
> - "NumElements field (%u), truncated",
> - i, element_count));
> + acpi_info("Actual Package length (%u) is larger than
> NumElements field (%u), truncated\n",
> + i, element_count);
> } else if (i < element_count) {
> /*
> * Arg list (elements) was exhausted, but we did not reach
> num_elements count.
> diff --git a/drivers/acpi/acpica/evgpeblk.c
> b/drivers/acpi/acpica/evgpeblk.c index 447fa1c..2244ed8 100644
> --- a/drivers/acpi/acpica/evgpeblk.c
> +++ b/drivers/acpi/acpica/evgpeblk.c
> @@ -499,10 +499,10 @@ acpi_ev_initialize_gpe_block(struct
> acpi_gpe_xrupt_info *gpe_xrupt_info,
> }
>
> if (gpe_enabled_count) {
> - ACPI_INFO(("Enabled %u GPEs in block %02X to %02X",
> - gpe_enabled_count, (u32)gpe_block->block_base_number,
> - (u32)(gpe_block->block_base_number +
> - (gpe_block->gpe_count - 1))));
> + acpi_info("Enabled %u GPEs in block %02X to %02X\n",
> + gpe_enabled_count, (u32)gpe_block->block_base_number,
> + (u32)(gpe_block->block_base_number +
> + gpe_block->gpe_count - 1));
> }
>
> gpe_block->initialized = TRUE;
> diff --git a/drivers/acpi/acpica/evgpeinit.c
> b/drivers/acpi/acpica/evgpeinit.c index 7dc7547..3a6d717 100644
> --- a/drivers/acpi/acpica/evgpeinit.c
> +++ b/drivers/acpi/acpica/evgpeinit.c
> @@ -281,7 +281,7 @@ void acpi_ev_update_gpes(acpi_owner_id table_owner_id)
> }
>
> if (walk_info.count) {
> - ACPI_INFO(("Enabled %u new GPEs", walk_info.count));
> + acpi_info("Enabled %u new GPEs\n", walk_info.count);
> }
>
> (void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
> diff --git a/drivers/acpi/acpica/exconfig.c
> b/drivers/acpi/acpica/exconfig.c index f741613..99916fe 100644
> --- a/drivers/acpi/acpica/exconfig.c
> +++ b/drivers/acpi/acpica/exconfig.c
> @@ -252,7 +252,7 @@ acpi_ex_load_table_op(struct acpi_walk_state
> *walk_state,
>
> status = acpi_get_table_by_index(table_index, &table);
> if (ACPI_SUCCESS(status)) {
> - ACPI_INFO(("Dynamic OEM Table Load:"));
> + acpi_info("Dynamic OEM Table Load:\n");
> acpi_tb_print_table_header(0, table);
> }
>
> @@ -472,7 +472,7 @@ acpi_ex_load_op(union acpi_operand_object *obj_desc,
>
> /* Install the new table into the local data structures */
>
> - ACPI_INFO(("Dynamic OEM Table Load:"));
> + acpi_info("Dynamic OEM Table Load:\n");
> (void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
>
> status = acpi_tb_install_standard_table(ACPI_PTR_TO_PHYSADDR(table),
> diff --git a/drivers/acpi/acpica/nseval.c b/drivers/acpi/acpica/nseval.c
> index 5d59cfc..18777f0 100644
> --- a/drivers/acpi/acpica/nseval.c
> +++ b/drivers/acpi/acpica/nseval.c
> @@ -378,8 +378,8 @@ void acpi_ns_exec_module_code_list(void)
> acpi_ut_remove_reference(prev);
> }
>
> - ACPI_INFO(("Executed %u blocks of module-level executable AML code",
> - method_count));
> + acpi_info("Executed %u blocks of module-level executable AML
> code\n",
> + method_count);
>
> ACPI_FREE(info);
> acpi_gbl_module_code_list = NULL;
> diff --git a/drivers/acpi/acpica/tbinstal.c
> b/drivers/acpi/acpica/tbinstal.c index 4dc6108..28f2ab7 100644
> --- a/drivers/acpi/acpica/tbinstal.c
> +++ b/drivers/acpi/acpica/tbinstal.c
> @@ -267,9 +267,9 @@ acpi_tb_install_standard_table(acpi_physical_address
> address,
> if (!reload &&
> acpi_gbl_disable_ssdt_table_install &&
> ACPI_COMPARE_NAME(&new_table_desc.signature, ACPI_SIG_SSDT)) {
> - ACPI_INFO(("Ignoring installation of %4.4s at %8.8X%8.8X",
> - new_table_desc.signature.ascii,
> - ACPI_FORMAT_UINT64(address)));
> + acpi_info("Ignoring installation of %4.4s at %8.8X%8.8X\n",
> + new_table_desc.signature.ascii,
> + ACPI_FORMAT_UINT64(address));
> goto release_and_exit;
> }
>
> @@ -431,11 +431,10 @@ finish_override:
> return;
> }
>
> - ACPI_INFO(("%4.4s 0x%8.8X%8.8X"
> - " %s table override, new table: 0x%8.8X%8.8X",
> - old_table_desc->signature.ascii,
> - ACPI_FORMAT_UINT64(old_table_desc->address),
> - override_type,
> ACPI_FORMAT_UINT64(new_table_desc.address)));
> + acpi_info("%4.4s 0x%8.8X%8.8X %s table override, new table:
> 0x%8.8X%8.8X\n",
> + old_table_desc->signature.ascii,
> + ACPI_FORMAT_UINT64(old_table_desc->address),
> + override_type, ACPI_FORMAT_UINT64(new_table_desc.address));
>
> /* We can now uninstall the original table */
>
> diff --git a/drivers/acpi/acpica/tbprint.c b/drivers/acpi/acpica/tbprint.c
> index 26d61db..030d610 100644
> --- a/drivers/acpi/acpica/tbprint.c
> +++ b/drivers/acpi/acpica/tbprint.c
> @@ -132,9 +132,9 @@ acpi_tb_print_table_header(acpi_physical_address
> address,
>
> /* FACS only has signature and length fields */
>
> - ACPI_INFO(("%-4.4s 0x%8.8X%8.8X %06X",
> - header->signature, ACPI_FORMAT_UINT64(address),
> - header->length));
> + acpi_info("%-4.4s 0x%8.8X%8.8X %06X\n",
> + header->signature, ACPI_FORMAT_UINT64(address),
> + header->length);
> } else if (ACPI_VALIDATE_RSDP_SIG(header->signature)) {
>
> /* RSDP has no common fields */
> @@ -144,28 +144,24 @@ acpi_tb_print_table_header(acpi_physical_address
> address,
> ACPI_OEM_ID_SIZE);
> acpi_tb_fix_string(local_header.oem_id, ACPI_OEM_ID_SIZE);
>
> - ACPI_INFO(("RSDP 0x%8.8X%8.8X %06X (v%.2d %-6.6s)",
> - ACPI_FORMAT_UINT64(address),
> - (ACPI_CAST_PTR(struct acpi_table_rsdp, header)->
> - revision >
> - 0) ? ACPI_CAST_PTR(struct acpi_table_rsdp,
> - header)->length : 20,
> - ACPI_CAST_PTR(struct acpi_table_rsdp,
> - header)->revision,
> - local_header.oem_id));
> + acpi_info("RSDP 0x%8.8X%8.8X %06X (v%.2d %-6.6s)\n",
> + ACPI_FORMAT_UINT64(address),
> + ACPI_CAST_PTR(struct acpi_table_rsdp, header)-
> >revision > 0 ?
> + ACPI_CAST_PTR(struct acpi_table_rsdp, header)->length
> : 20,
> + ACPI_CAST_PTR(struct acpi_table_rsdp, header)-
> >revision,
> + local_header.oem_id);
> } else {
> /* Standard ACPI table with full common header */
>
> acpi_tb_cleanup_table_header(&local_header, header);
>
> - ACPI_INFO(("%-4.4s 0x%8.8X%8.8X"
> - " %06X (v%.2d %-6.6s %-8.8s %08X %-4.4s %08X)",
> - local_header.signature, ACPI_FORMAT_UINT64(address),
> - local_header.length, local_header.revision,
> - local_header.oem_id, local_header.oem_table_id,
> - local_header.oem_revision,
> - local_header.asl_compiler_id,
> - local_header.asl_compiler_revision));
> + acpi_info("%-4.4s 0x%8.8X%8.8X %06X (v%.2d %-6.6s %-8.8s %08X
> %-4.4s %08X)\n",
> + local_header.signature, ACPI_FORMAT_UINT64(address),
> + local_header.length, local_header.revision,
> + local_header.oem_id, local_header.oem_table_id,
> + local_header.oem_revision,
> + local_header.asl_compiler_id,
> + local_header.asl_compiler_revision);
> }
> }
>
> diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
> index 9240c76..8f64b38 100644
> --- a/drivers/acpi/acpica/tbutils.c
> +++ b/drivers/acpi/acpica/tbutils.c
> @@ -174,7 +174,8 @@ struct acpi_table_header *acpi_tb_copy_dsdt(u32
> table_index)
> ACPI_TABLE_ORIGIN_INTERNAL_VIRTUAL,
> new_table);
>
> - ACPI_INFO(("Forced DSDT copy: length 0x%05X copied locally, original
> unmapped", new_table->length));
> + acpi_info("Forced DSDT copy: length 0x%05X copied locally, original
> unmapped\n",
> + new_table->length);
>
> return (new_table);
> }
> diff --git a/drivers/acpi/acpica/tbxfload.c
> b/drivers/acpi/acpica/tbxfload.c index 3151968..0827d17 100644
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -240,7 +240,8 @@ 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\n",
> + tables_loaded);
> } else {
> ACPI_ERROR((AE_INFO,
> "%u table load failures, %u successful", @@ -333,7
> +334,7 @@ acpi_status acpi_load_table(struct acpi_table_header *table)
>
> /* Install the table and load it into the namespace */
>
> - ACPI_INFO(("Host-directed Dynamic ACPI Table Load:"));
> + acpi_info("Host-directed Dynamic ACPI Table Load:\n");
> (void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
>
> status = acpi_tb_install_standard_table(ACPI_PTR_TO_PHYSADDR(table),
> diff --git a/drivers/acpi/acpica/uttrack.c b/drivers/acpi/acpica/uttrack.c
> index 60c406a..b1d6f4a 100644
> --- a/drivers/acpi/acpica/uttrack.c
> +++ b/drivers/acpi/acpica/uttrack.c
> @@ -712,7 +712,7 @@ void acpi_ut_dump_allocations(u32 component, const
> char *module)
> /* Print summary */
>
> if (!num_outstanding) {
> - ACPI_INFO(("No outstanding allocations"));
> + acpi_info("No outstanding allocations\n");
> } else {
> ACPI_ERROR((AE_INFO, "%u(0x%X) Outstanding allocations",
> num_outstanding, num_outstanding)); diff --git
> a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c index
> d9f15cb..e9a80c2 100644
> --- a/drivers/acpi/acpica/utxferror.c
> +++ b/drivers/acpi/acpica/utxferror.c
> @@ -161,7 +161,7 @@ ACPI_EXPORT_SYMBOL(acpi_warning)
>
> /************************************************************************
> *******
> *
> - * FUNCTION: acpi_info
> + * FUNCTION: _acpi_info
> *
> * PARAMETERS: module_name - Caller's module name (for error
> output)
> * line_number - Caller's line number (for error
> output) @@ -175,7 +175,7 @@ ACPI_EXPORT_SYMBOL(acpi_warning)
> * TBD: module_name and line_number args are not needed, should be
> removed.
> *
> ************************************************************************
> ******/
> -void ACPI_INTERNAL_VAR_XFACE acpi_info(const char *format, ...)
> +void ACPI_INTERNAL_VAR_XFACE _acpi_info(const char *format, ...)
> {
> va_list arg_list;
>
> @@ -184,13 +184,12 @@ void ACPI_INTERNAL_VAR_XFACE acpi_info(const char
> *format, ...)
>
> va_start(arg_list, format);
> acpi_os_vprintf(format, arg_list);
> - acpi_os_printf("\n");
> va_end(arg_list);
>
> ACPI_MSG_REDIRECT_END;
> }
>
> -ACPI_EXPORT_SYMBOL(acpi_info)
> +ACPI_EXPORT_SYMBOL(_acpi_info)
>
> /************************************************************************
> *******
> *
> diff --git a/include/acpi/acoutput.h b/include/acpi/acoutput.h index
> 34f601e..4812992 100644
> --- a/include/acpi/acoutput.h
> +++ b/include/acpi/acoutput.h
> @@ -223,10 +223,9 @@
>
> /*
> * 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_info(fmt, ...) _acpi_info(fmt, ##__VA_ARGS__)
> #define ACPI_WARNING(plist) acpi_warning plist
> #define ACPI_EXCEPTION(plist) acpi_exception plist
> #define ACPI_ERROR(plist) acpi_error plist @@ -238,7 +237,8
> @@
>
> /* No error messages */
>
> -#define ACPI_INFO(plist)
> +#define acpi_info(fmt, ...) \
> + do { if (0) _acpi_info(fmt, ##__VA_ARGS__); } while (0)
> #define ACPI_WARNING(plist)
> #define ACPI_EXCEPTION(plist)
> #define ACPI_ERROR(plist)
> diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h index
> 1755697..f5f96ac 100644
> --- a/include/acpi/acpixf.h
> +++ b/include/acpi/acpixf.h
> @@ -899,7 +899,7 @@ ACPI_MSG_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(3)
> const char *format, ...))
> ACPI_MSG_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(1)
> void ACPI_INTERNAL_VAR_XFACE
> - acpi_info(const char *format, ...))
> + _acpi_info(const char *format, ...))
> ACPI_MSG_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(3)
> void ACPI_INTERNAL_VAR_XFACE
> acpi_bios_error(const char *module_name,