RE: [RFC PATCH] acpi: Use a more normal logging style for ACPI_<LEVEL> calls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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,
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux