xen/drivers/acpi/tables/tbxface.c | 4 ++-- xen/include/acpi/acpixf.h | 13 ++----------- xen/include/acpi/actypes.h | 1 - 3 files changed, 4 insertions(+), 14 deletions(-)
Typedef-ing a naked pointer like this an anti-pattern which is best avoided.
Furthermore, it's bad to pass a string literate in a mutable type. Delete the
type entirely, and replace it with a plain 'const char *'.
This highlights two futher bugs. acpi_get_table() already had a mismatch in
types between it's declaration and definition, and we have declarations for
acpi_get_handle() and acpi_get_table_header() but no definition at all (nor
any callers).
This fixes violations of MISRA Rule 7.4:
A string literal shall not be assigned to an object unless the object's type
is "pointer to const-qualified char".
and of Rule 8.3:
All declarations of an object or function shall use the same names and type
qualifiers.
and of Rule 8.6:
An identifier with external linkage shall have exactly one external
definition.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
CC: Julien Grall <julien@xen.org>
CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
Roberto/Nicola: Please double check my choice of rules here, and point out any
others that I may have missed.
---
xen/drivers/acpi/tables/tbxface.c | 4 ++--
xen/include/acpi/acpixf.h | 13 ++-----------
xen/include/acpi/actypes.h | 1 -
3 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/xen/drivers/acpi/tables/tbxface.c b/xen/drivers/acpi/tables/tbxface.c
index 21b2e5eae1c7..204d66caea48 100644
--- a/xen/drivers/acpi/tables/tbxface.c
+++ b/xen/drivers/acpi/tables/tbxface.c
@@ -164,7 +164,7 @@ acpi_initialize_tables(struct acpi_table_desc * initial_table_array,
*
*****************************************************************************/
acpi_status __init
-acpi_get_table(char *signature,
+acpi_get_table(const char *signature,
acpi_native_uint instance, struct acpi_table_header **out_table)
{
acpi_native_uint i;
@@ -220,7 +220,7 @@ acpi_get_table(char *signature,
*
*****************************************************************************/
acpi_status __init
-acpi_get_table_phys(acpi_string signature, acpi_native_uint instance,
+acpi_get_table_phys(const char *signature, acpi_native_uint instance,
acpi_physical_address *addr, acpi_native_uint *len)
{
acpi_native_uint i, j;
diff --git a/xen/include/acpi/acpixf.h b/xen/include/acpi/acpixf.h
index ba74908f0478..8b70154b8f96 100644
--- a/xen/include/acpi/acpixf.h
+++ b/xen/include/acpi/acpixf.h
@@ -69,25 +69,16 @@ acpi_status acpi_load_tables(void);
acpi_status acpi_load_table(struct acpi_table_header *table_ptr);
acpi_status
-acpi_get_table_header(acpi_string signature,
- acpi_native_uint instance,
- struct acpi_table_header *out_table_header);
-
-acpi_status
-acpi_get_table(acpi_string signature,
+acpi_get_table(const char *signature,
acpi_native_uint instance, struct acpi_table_header **out_table);
acpi_status
-acpi_get_table_phys(acpi_string signature, acpi_native_uint instance,
+acpi_get_table_phys(const char *signature, acpi_native_uint instance,
acpi_physical_address *addr, acpi_native_uint *len);
/*
* Namespace and name interfaces
*/
acpi_status
-acpi_get_handle(acpi_handle parent,
- acpi_string pathname, acpi_handle * ret_handle);
-
-acpi_status
acpi_debug_trace(char *name, u32 debug_level, u32 debug_layer, u32 flags);
acpi_status
diff --git a/xen/include/acpi/actypes.h b/xen/include/acpi/actypes.h
index f3e95abc3ab3..7023863d0349 100644
--- a/xen/include/acpi/actypes.h
+++ b/xen/include/acpi/actypes.h
@@ -281,7 +281,6 @@ typedef acpi_native_uint acpi_size;
*/
typedef u32 acpi_status; /* All ACPI Exceptions */
typedef u32 acpi_name; /* 4-byte ACPI name */
-typedef char *acpi_string; /* Null terminated ASCII string */
typedef void *acpi_handle; /* Actually a ptr to a NS Node */
struct uint64_struct {
--
2.30.2On 14.07.2023 15:04, Andrew Cooper wrote: > Typedef-ing a naked pointer like this an anti-pattern which is best avoided. > Furthermore, it's bad to pass a string literate in a mutable type. Delete the > type entirely, and replace it with a plain 'const char *'. > > This highlights two futher bugs. acpi_get_table() already had a mismatch in > types between it's declaration and definition, and we have declarations for > acpi_get_handle() and acpi_get_table_header() but no definition at all (nor > any callers). > > This fixes violations of MISRA Rule 7.4: > > A string literal shall not be assigned to an object unless the object's type > is "pointer to const-qualified char". > > and of Rule 8.3: > > All declarations of an object or function shall use the same names and type > qualifiers. > > and of Rule 8.6: > > An identifier with external linkage shall have exactly one external > definition. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
On 14/07/23 15:04, Andrew Cooper wrote:
> Typedef-ing a naked pointer like this an anti-pattern which is best avoided.
s/this an/this is an/
> Furthermore, it's bad to pass a string literate in a mutable type. Delete the
s/literate/literal/
> type entirely, and replace it with a plain 'const char *'.
>
> This highlights two futher bugs. acpi_get_table() already had a mismatch in
> types between it's declaration and definition, and we have declarations for
> acpi_get_handle() and acpi_get_table_header() but no definition at all (nor
> any callers).
>
> This fixes violations of MISRA Rule 7.4:
>
> A string literal shall not be assigned to an object unless the object's type
> is "pointer to const-qualified char".
>
> and of Rule 8.3:
>
> All declarations of an object or function shall use the same names and type
> qualifiers.
>
> and of Rule 8.6:
>
> An identifier with external linkage shall have exactly one external
> definition.
The choice of rules looks good to me, but perhaps Roberto has some
additional insight on this.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Wei Liu <wl@xen.org>
> CC: Julien Grall <julien@xen.org>
> CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
>
> Roberto/Nicola: Please double check my choice of rules here, and point out any
> others that I may have missed.
> ---
> xen/drivers/acpi/tables/tbxface.c | 4 ++--
> xen/include/acpi/acpixf.h | 13 ++-----------
> xen/include/acpi/actypes.h | 1 -
> 3 files changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/xen/drivers/acpi/tables/tbxface.c b/xen/drivers/acpi/tables/tbxface.c
> index 21b2e5eae1c7..204d66caea48 100644
> --- a/xen/drivers/acpi/tables/tbxface.c
> +++ b/xen/drivers/acpi/tables/tbxface.c
> @@ -164,7 +164,7 @@ acpi_initialize_tables(struct acpi_table_desc * initial_table_array,
> *
> *****************************************************************************/
> acpi_status __init
> -acpi_get_table(char *signature,
> +acpi_get_table(const char *signature,
> acpi_native_uint instance, struct acpi_table_header **out_table)
> {
> acpi_native_uint i;
> @@ -220,7 +220,7 @@ acpi_get_table(char *signature,
> *
> *****************************************************************************/
> acpi_status __init
> -acpi_get_table_phys(acpi_string signature, acpi_native_uint instance,
> +acpi_get_table_phys(const char *signature, acpi_native_uint instance,
> acpi_physical_address *addr, acpi_native_uint *len)
> {
> acpi_native_uint i, j;
> diff --git a/xen/include/acpi/acpixf.h b/xen/include/acpi/acpixf.h
> index ba74908f0478..8b70154b8f96 100644
> --- a/xen/include/acpi/acpixf.h
> +++ b/xen/include/acpi/acpixf.h
> @@ -69,25 +69,16 @@ acpi_status acpi_load_tables(void);
> acpi_status acpi_load_table(struct acpi_table_header *table_ptr);
>
> acpi_status
> -acpi_get_table_header(acpi_string signature,
> - acpi_native_uint instance,
> - struct acpi_table_header *out_table_header);
> -
> -acpi_status
> -acpi_get_table(acpi_string signature,
> +acpi_get_table(const char *signature,
> acpi_native_uint instance, struct acpi_table_header **out_table);
>
> acpi_status
> -acpi_get_table_phys(acpi_string signature, acpi_native_uint instance,
> +acpi_get_table_phys(const char *signature, acpi_native_uint instance,
> acpi_physical_address *addr, acpi_native_uint *len);
> /*
> * Namespace and name interfaces
> */
> acpi_status
> -acpi_get_handle(acpi_handle parent,
> - acpi_string pathname, acpi_handle * ret_handle);
> -
> -acpi_status
> acpi_debug_trace(char *name, u32 debug_level, u32 debug_layer, u32 flags);
>
> acpi_status
> diff --git a/xen/include/acpi/actypes.h b/xen/include/acpi/actypes.h
> index f3e95abc3ab3..7023863d0349 100644
> --- a/xen/include/acpi/actypes.h
> +++ b/xen/include/acpi/actypes.h
> @@ -281,7 +281,6 @@ typedef acpi_native_uint acpi_size;
> */
> typedef u32 acpi_status; /* All ACPI Exceptions */
> typedef u32 acpi_name; /* 4-byte ACPI name */
> -typedef char *acpi_string; /* Null terminated ASCII string */
> typedef void *acpi_handle; /* Actually a ptr to a NS Node */
>
> struct uint64_struct {
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
On 14/07/2023 2:44 pm, Nicola Vetrini wrote: > > On 14/07/23 15:04, Andrew Cooper wrote: >> Typedef-ing a naked pointer like this an anti-pattern which is best >> avoided. > > s/this an/this is an/ > >> Furthermore, it's bad to pass a string literate in a mutable type. >> Delete the > > s/literate/literal/ Fixed, thanks. And the 3rd typo below. > >> type entirely, and replace it with a plain 'const char *'. >> >> This highlights two futher bugs. acpi_get_table() already had a >> mismatch in >> types between it's declaration and definition, and we have >> declarations for >> acpi_get_handle() and acpi_get_table_header() but no definition at >> all (nor >> any callers). >> >> This fixes violations of MISRA Rule 7.4: >> >> A string literal shall not be assigned to an object unless the >> object's type >> is "pointer to const-qualified char". >> >> and of Rule 8.3: >> >> All declarations of an object or function shall use the same names >> and type >> qualifiers. >> >> and of Rule 8.6: >> >> An identifier with external linkage shall have exactly one external >> definition. > > The choice of rules looks good to me, but perhaps Roberto has some > additional insight on this. Thanks. ~Andrew
© 2016 - 2024 Red Hat, Inc.