Re: [PATCH] hv: treewide: fix Macro redefine, usage -- and operation violations


Junjie Mao
 

junjunshan1 <junjun.shan@...> writes:

MISARC has requirements about Marco redefinition, usage of ++ or -- and assignment operator in boolean expression.
This patch is used to solve these violations.

The modifications are summarized as following:
1.The HC_VM_SET_MEMORY_REGION, HC_VM_GPA2HPA, HC_VM_SET_MEMORY_REGIONS are redefined twice in acrn_hv_des.h, so delete them to solve the macro redefinition violations.
2.The macro BUS_LOCK are redefined in bits.h and atomic.h, then delete the declaration in both two files, add a new declaration in cpu.h and include the header file.
3.modify the code to deprecate the usage of -- operators in string.c.
4.modify the while loop to for loop to avoid assignment operator in
boolean expression in vlapic.c.
Some general conventions about commit logs:

1. Each line should be no more than 80 characters. Break the
paragraph if it is longer than that.

2. Due to the first rule, there typically is an empty line between
adjacent paragraphs/items.


Signed-off-by: junjunshan1 <junjun.shan@...>
Reviewed-by: Junjie Mao <junjie.mao@...>

With a minor style issue below.

Please also set your name properly by executing 'git config --global
user.name <your name>'.

---
hypervisor/arch/x86/guest/vlapic.c | 3 ++-
hypervisor/include/arch/x86/cpu.h | 2 ++
hypervisor/include/lib/atomic.h | 3 +--
hypervisor/include/lib/bits.h | 3 +--
hypervisor/include/public/acrn_hv_defs.h | 3 ---
hypervisor/lib/string.c | 6 ++++--
6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c
index 35be79c..72acb76 100644
--- a/hypervisor/arch/x86/guest/vlapic.c
+++ b/hypervisor/arch/x86/guest/vlapic.c
@@ -1135,7 +1135,8 @@ vlapic_icrlo_write_handler(struct acrn_vlapic *vlapic)
break;
}

- while ((vcpu_id = ffs64(dmask)) != INVALID_BIT_INDEX) {
+ for (vcpu_id = ffs64(dmask); vcpu_id != INVALID_BIT_INDEX;
+ vcpu_id = ffs64(dmask)) {
bitmap_clear_lock(vcpu_id, &dmask);
target_vcpu = vcpu_from_vid(vlapic->vm, vcpu_id);
if (target_vcpu == NULL) {
diff --git a/hypervisor/include/arch/x86/cpu.h b/hypervisor/include/arch/x86/cpu.h
index e56f443..d24228d 100644
--- a/hypervisor/include/arch/x86/cpu.h
+++ b/hypervisor/include/arch/x86/cpu.h
@@ -152,6 +152,8 @@

#ifndef ASSEMBLER

+#define BUS_LOCK "lock ; "
+
/**
*
* Identifiers for architecturally defined registers.
diff --git a/hypervisor/include/lib/atomic.h b/hypervisor/include/lib/atomic.h
index 264917c..6085863 100644
--- a/hypervisor/include/lib/atomic.h
+++ b/hypervisor/include/lib/atomic.h
@@ -29,8 +29,7 @@

#ifndef ATOMIC_H
#define ATOMIC_H
-
-#define BUS_LOCK "lock ; "
+#include <cpu.h>

#define build_atomic_load(name, size, type, ptr) \
static inline type name(const volatile type *ptr) \
diff --git a/hypervisor/include/lib/bits.h b/hypervisor/include/lib/bits.h
index 04a5057..ea0e8a1 100644
--- a/hypervisor/include/lib/bits.h
+++ b/hypervisor/include/lib/bits.h
@@ -29,8 +29,7 @@

#ifndef BITS_H
#define BITS_H
-
-#define BUS_LOCK "lock ; "
+#include <cpu.h>
/**
*
* INVALID_BIT_INDEX means when input paramter is zero,
diff --git a/hypervisor/include/public/acrn_hv_defs.h b/hypervisor/include/public/acrn_hv_defs.h
index cfeb834..700ce92 100644
--- a/hypervisor/include/public/acrn_hv_defs.h
+++ b/hypervisor/include/public/acrn_hv_defs.h
@@ -55,9 +55,6 @@
#define HC_VM_GPA2HPA BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x01UL)
#define HC_VM_SET_MEMORY_REGIONS BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x02UL)
#define HC_VM_WRITE_PROTECT_PAGE BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x03UL)
-#define HC_VM_SET_MEMORY_REGION BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x00UL)
-#define HC_VM_GPA2HPA BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x01UL)
-#define HC_VM_SET_MEMORY_REGIONS BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x02UL)

/* PCI assignment*/
#define HC_ID_PCI_BASE 0x50UL
diff --git a/hypervisor/lib/string.c b/hypervisor/lib/string.c
index db4f145..7d2a5a5 100644
--- a/hypervisor/lib/string.c
+++ b/hypervisor/lib/string.c
@@ -210,7 +210,8 @@ char *strcpy_s(char *d_arg, size_t dmax, const char *s_arg)
while (dest_avail > 0U) {
if (overlap_guard == 0U) {
pr_err("%s: overlap happened.", __func__);
- *(--d) = '\0';
+ d = d-1;
A single space is required between a binary operator and either of its
operand per our coding style.

+ *d = '\0';
return NULL;
}

@@ -293,7 +294,8 @@ char *strncpy_s(char *d_arg, size_t dmax, const char *s_arg, size_t slen_arg)
while (dest_avail > 0U) {
if (overlap_guard == 0U) {
pr_err("%s: overlap happened.", __func__);
- *(--d) = '\0';
+ d = d-1;
Ditto.

--
Best Regards
Junjie Mao

+ *d = '\0';
return NULL;
}

Join acrn-dev@lists.projectacrn.org to automatically receive all group messages.