Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
---
migration/migration.c | 4 ++--
migration/postcopy-ram.c | 2 +-
migration/ram.c | 2 +-
migration/savevm.c | 2 +-
migration/vmstate.c | 10 +++++-----
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 0575ecb..e050f57 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2478,8 +2478,8 @@ static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname,
* Since we currently insist on matching page sizes, just sanity check
* we're being asked for whole host pages.
*/
- if (start & (our_host_ps-1) ||
- (len & (our_host_ps-1))) {
+ if (start & (our_host_ps - 1) ||
+ (len & (our_host_ps - 1))) {
error_report("%s: Misaligned page request, start: " RAM_ADDR_FMT
" len: %zd", __func__, start, len);
mark_source_rp_bad(ms);
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 0a2f88a8..eea92bb 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -403,7 +403,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
strerror(errno));
goto out;
}
- g_assert(((size_t)testarea & (pagesize-1)) == 0);
+ g_assert(((size_t)testarea & (pagesize - 1)) == 0);
reg_struct.range.start = (uintptr_t)testarea;
reg_struct.range.len = pagesize;
diff --git a/migration/ram.c b/migration/ram.c
index 6ed4f9e..0aea78f 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1563,7 +1563,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len)
rs->last_req_rb = ramblock;
}
trace_ram_save_queue_pages(ramblock->idstr, start, len);
- if (start+len > ramblock->used_length) {
+ if (start + len > ramblock->used_length) {
error_report("%s request overrun start=" RAM_ADDR_FMT " len="
RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
__func__, start, len, ramblock->used_length);
diff --git a/migration/savevm.c b/migration/savevm.c
index d2e141f..9e95df1 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -521,7 +521,7 @@ static const VMStateDescription vmstate_configuration = {
VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, len),
VMSTATE_END_OF_LIST()
},
- .subsections = (const VMStateDescription*[]) {
+ .subsections = (const VMStateDescription * []) {
&vmstate_target_page_bits,
&vmstate_capabilites,
&vmstate_uuid,
diff --git a/migration/vmstate.c b/migration/vmstate.c
index bafa890..e9d2aef 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -32,13 +32,13 @@ static int vmstate_n_elems(void *opaque, const VMStateField *field)
if (field->flags & VMS_ARRAY) {
n_elems = field->num;
} else if (field->flags & VMS_VARRAY_INT32) {
- n_elems = *(int32_t *)(opaque+field->num_offset);
+ n_elems = *(int32_t *)(opaque + field->num_offset);
} else if (field->flags & VMS_VARRAY_UINT32) {
- n_elems = *(uint32_t *)(opaque+field->num_offset);
+ n_elems = *(uint32_t *)(opaque + field->num_offset);
} else if (field->flags & VMS_VARRAY_UINT16) {
- n_elems = *(uint16_t *)(opaque+field->num_offset);
+ n_elems = *(uint16_t *)(opaque + field->num_offset);
} else if (field->flags & VMS_VARRAY_UINT8) {
- n_elems = *(uint8_t *)(opaque+field->num_offset);
+ n_elems = *(uint8_t *)(opaque + field->num_offset);
}
if (field->flags & VMS_MULTIPLY_ELEMENTS) {
@@ -54,7 +54,7 @@ static int vmstate_size(void *opaque, const VMStateField *field)
int size = field->size;
if (field->flags & VMS_VBUFFER) {
- size = *(int32_t *)(opaque+field->size_offset);
+ size = *(int32_t *)(opaque + field->size_offset);
if (field->flags & VMS_MULTIPLY) {
size *= field->size;
}
--
1.8.3.1
* Bihong Yu (yubihong@huawei.com) wrote:
> Signed-off-by: Bihong Yu <yubihong@huawei.com>
> Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
Yes that's OK, I'm a bit sturprised we need the space afte rthe * in the
VMStateDescription case, I wouldn't necessarily go and change them all.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/migration.c | 4 ++--
> migration/postcopy-ram.c | 2 +-
> migration/ram.c | 2 +-
> migration/savevm.c | 2 +-
> migration/vmstate.c | 10 +++++-----
> 5 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 0575ecb..e050f57 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2478,8 +2478,8 @@ static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname,
> * Since we currently insist on matching page sizes, just sanity check
> * we're being asked for whole host pages.
> */
> - if (start & (our_host_ps-1) ||
> - (len & (our_host_ps-1))) {
> + if (start & (our_host_ps - 1) ||
> + (len & (our_host_ps - 1))) {
> error_report("%s: Misaligned page request, start: " RAM_ADDR_FMT
> " len: %zd", __func__, start, len);
> mark_source_rp_bad(ms);
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 0a2f88a8..eea92bb 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -403,7 +403,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
> strerror(errno));
> goto out;
> }
> - g_assert(((size_t)testarea & (pagesize-1)) == 0);
> + g_assert(((size_t)testarea & (pagesize - 1)) == 0);
>
> reg_struct.range.start = (uintptr_t)testarea;
> reg_struct.range.len = pagesize;
> diff --git a/migration/ram.c b/migration/ram.c
> index 6ed4f9e..0aea78f 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1563,7 +1563,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len)
> rs->last_req_rb = ramblock;
> }
> trace_ram_save_queue_pages(ramblock->idstr, start, len);
> - if (start+len > ramblock->used_length) {
> + if (start + len > ramblock->used_length) {
> error_report("%s request overrun start=" RAM_ADDR_FMT " len="
> RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
> __func__, start, len, ramblock->used_length);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index d2e141f..9e95df1 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -521,7 +521,7 @@ static const VMStateDescription vmstate_configuration = {
> VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, len),
> VMSTATE_END_OF_LIST()
> },
> - .subsections = (const VMStateDescription*[]) {
> + .subsections = (const VMStateDescription * []) {
> &vmstate_target_page_bits,
> &vmstate_capabilites,
> &vmstate_uuid,
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index bafa890..e9d2aef 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -32,13 +32,13 @@ static int vmstate_n_elems(void *opaque, const VMStateField *field)
> if (field->flags & VMS_ARRAY) {
> n_elems = field->num;
> } else if (field->flags & VMS_VARRAY_INT32) {
> - n_elems = *(int32_t *)(opaque+field->num_offset);
> + n_elems = *(int32_t *)(opaque + field->num_offset);
> } else if (field->flags & VMS_VARRAY_UINT32) {
> - n_elems = *(uint32_t *)(opaque+field->num_offset);
> + n_elems = *(uint32_t *)(opaque + field->num_offset);
> } else if (field->flags & VMS_VARRAY_UINT16) {
> - n_elems = *(uint16_t *)(opaque+field->num_offset);
> + n_elems = *(uint16_t *)(opaque + field->num_offset);
> } else if (field->flags & VMS_VARRAY_UINT8) {
> - n_elems = *(uint8_t *)(opaque+field->num_offset);
> + n_elems = *(uint8_t *)(opaque + field->num_offset);
> }
>
> if (field->flags & VMS_MULTIPLY_ELEMENTS) {
> @@ -54,7 +54,7 @@ static int vmstate_size(void *opaque, const VMStateField *field)
> int size = field->size;
>
> if (field->flags & VMS_VBUFFER) {
> - size = *(int32_t *)(opaque+field->size_offset);
> + size = *(int32_t *)(opaque + field->size_offset);
> if (field->flags & VMS_MULTIPLY) {
> size *= field->size;
> }
> --
> 1.8.3.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> * Bihong Yu (yubihong@huawei.com) wrote:
>> Signed-off-by: Bihong Yu <yubihong@huawei.com>
>> Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
>
> Yes that's OK, I'm a bit sturprised we need the space afte rthe * in the
> VMStateDescription case, I wouldn't necessarily go and change them all.
We don't: it's not the binary multiplication operator *, where we want a
space on both sides, it's a pointer declarator, where we want a space on
the left only.
Example:
int *pa, *pb, *pc;
*pa = *pb * *pc;
Note the space on both side of binary operator * (multiplication), but
only on the left side of the pointer declarator's * and the unary
operator * (indirection).
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
[...]
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index d2e141f..9e95df1 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -521,7 +521,7 @@ static const VMStateDescription vmstate_configuration = {
>> VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, len),
>> VMSTATE_END_OF_LIST()
>> },
>> - .subsections = (const VMStateDescription*[]) {
>> + .subsections = (const VMStateDescription * []) {
>> &vmstate_target_page_bits,
>> &vmstate_capabilites,
>> &vmstate_uuid,
Should be
.subsections = (const VMStateDescription *[]) {
[...]
Yes, I used to think "const VMStateDescription *[]" was right, but when I search
similar expressions, most of all are "xxx * []". Such as:
fsdev/qemu-fsdev.c:54: .opts = (const char * [])
hw/intc/s390_flic_kvm.c:567: .subsections = (const VMStateDescription * [])
...
So, I keep the same style. Should I change it to "const VMStateDescription *[]"?
On 2020/10/19 16:24, Markus Armbruster wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>
>> * Bihong Yu (yubihong@huawei.com) wrote:
>>> Signed-off-by: Bihong Yu <yubihong@huawei.com>
>>> Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
>>
>> Yes that's OK, I'm a bit sturprised we need the space afte rthe * in the
>> VMStateDescription case, I wouldn't necessarily go and change them all.
>
> We don't: it's not the binary multiplication operator *, where we want a
> space on both sides, it's a pointer declarator, where we want a space on
> the left only.
>
> Example:
>
> int *pa, *pb, *pc;
> *pa = *pb * *pc;
>
> Note the space on both side of binary operator * (multiplication), but
> only on the left side of the pointer declarator's * and the unary
> operator * (indirection).
>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> [...]
>>> diff --git a/migration/savevm.c b/migration/savevm.c
>>> index d2e141f..9e95df1 100644
>>> --- a/migration/savevm.c
>>> +++ b/migration/savevm.c
>>> @@ -521,7 +521,7 @@ static const VMStateDescription vmstate_configuration = {
>>> VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, len),
>>> VMSTATE_END_OF_LIST()
>>> },
>>> - .subsections = (const VMStateDescription*[]) {
>>> + .subsections = (const VMStateDescription * []) {
>>> &vmstate_target_page_bits,
>>> &vmstate_capabilites,
>>> &vmstate_uuid,
>
> Should be
>
> .subsections = (const VMStateDescription *[]) {
>
> [...]
>
> .
>
Bihong Yu <yubihong@huawei.com> writes: > Yes, I used to think "const VMStateDescription *[]" was right, but when I search > similar expressions, most of all are "xxx * []". Such as: > fsdev/qemu-fsdev.c:54: .opts = (const char * []) > hw/intc/s390_flic_kvm.c:567: .subsections = (const VMStateDescription * []) > ... All three variations occur in the code: no space, space on both sides, space only on the left. > So, I keep the same style. Should I change it to "const VMStateDescription *[]"? Dropping the change to savevm.c should be fine. Changing it to "VMStateDescription *[]" should be also fine. I figure you can keep David's R-by in both cases. [...]
OK, I will change it to "VMStateDescription *[]". Thank you for your review. On 2020/10/19 19:59, Markus Armbruster wrote: > Bihong Yu <yubihong@huawei.com> writes: > >> Yes, I used to think "const VMStateDescription *[]" was right, but when I search >> similar expressions, most of all are "xxx * []". Such as: >> fsdev/qemu-fsdev.c:54: .opts = (const char * []) >> hw/intc/s390_flic_kvm.c:567: .subsections = (const VMStateDescription * []) >> ... > > All three variations occur in the code: no space, space on both sides, > space only on the left. > >> So, I keep the same style. Should I change it to "const VMStateDescription *[]"? > > Dropping the change to savevm.c should be fine. > > Changing it to "VMStateDescription *[]" should be also fine. > > I figure you can keep David's R-by in both cases. > > [...] > > . >
© 2016 - 2026 Red Hat, Inc.