Re: [PATCH v2 3/8] migration: Add spaces around operator

2020-10-19 Thread Bihong Yu 
OK, I will change it to "VMStateDescription *[]". Thank you for your review.

On 2020/10/19 19:59, Markus Armbruster wrote:
> Bihong Yu  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.
> 
> [...]
> 
> .
>

Re: [PATCH v2 3/8] migration: Add spaces around operator

2020-10-19 Thread Markus Armbruster 
Bihong Yu  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.

[...]

Re: [PATCH v2 3/8] migration: Add spaces around operator

2020-10-19 Thread Bihong Yu 
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"  writes:
> 
>> * Bihong Yu (yubih...@huawei.com) wrote:
>>> Signed-off-by: Bihong Yu 
>>> Reviewed-by: Chuan Zheng 
>>
>> 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 
> [...]
>>> 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 *[]) {
> 
> [...]
> 
> .
>

Re: [PATCH v2 3/8] migration: Add spaces around operator

2020-10-19 Thread Markus Armbruster 
"Dr. David Alan Gilbert"  writes:

> * Bihong Yu (yubih...@huawei.com) wrote:
>> Signed-off-by: Bihong Yu 
>> Reviewed-by: Chuan Zheng 
>
> 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 
[...]
>> 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 *[]) {

[...]

Re: [PATCH v2 3/8] migration: Add spaces around operator

2020-10-13 Thread Dr. David Alan Gilbert 
* Bihong Yu (yubih...@huawei.com) wrote:
> Signed-off-by: Bihong Yu 
> Reviewed-by: Chuan Zheng 

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 

> ---
>  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 / dgilb...@redhat.com / Manchester, UK

[PATCH v2 3/8] migration: Add spaces around operator

2020-10-12 Thread Bihong Yu 
Signed-off-by: Bihong Yu 
Reviewed-by: Chuan Zheng 
---
 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