Re: [PATCH v2 6/9] dump: Introduce dump_is_64bit() helper function

[Richard Henderson]

On 3/10/22 03:08, Janosch Frank wrote:

Checking d_class in dump_info leads to lengthy conditionals so let's
shorten things a bit by introducing a helper function.

Signed-off-by: Janosch Frank 
---
  dump/dump.c | 19 ---
  1 file changed, 12 insertions(+), 7 deletions(-)


Reviewed-by: Richard Henderson 


@@ -1007,7 +1012,7 @@ out:
  
  static void write_dump_header(DumpState *s, Error **errp)

  {
-if (s->dump_info.d_class == ELFCLASS32) {
+if (!dump_is_64bit(s)) {
  create_header32(s, errp);
  } else {
  create_header64(s, errp);
@@ -1697,7 +1702,7 @@ static void dump_init(DumpState *s, int fd, bool 
has_format,
  uint32_t size;
  uint16_t format;
  
-note_head_size = s->dump_info.d_class == ELFCLASS32 ?

+note_head_size = !dump_is_64bit(s) ?
  sizeof(Elf32_Nhdr) : sizeof(Elf64_Nhdr);


It would be nice to standardize on positive tests, which in this case would reverse these 
two conditionals.



r~

[PATCH v2 6/9] dump: Introduce dump_is_64bit() helper function

[Janosch Frank]

Checking d_class in dump_info leads to lengthy conditionals so let's
shorten things a bit by introducing a helper function.

Signed-off-by: Janosch Frank 
---
 dump/dump.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index 12b3a1a83e..8fcd23571f 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -55,6 +55,11 @@ static Error *dump_migration_blocker;
   DIV_ROUND_UP((name_size), 4) +\
   DIV_ROUND_UP((desc_size), 4)) * 4)
 
+static inline bool dump_is_64bit(DumpState *s)
+{
+return s->dump_info.d_class == ELFCLASS64;
+}
+
 uint16_t cpu_to_dump16(DumpState *s, uint16_t val)
 {
 if (s->dump_info.d_endian == ELFDATA2LSB) {
@@ -479,7 +484,7 @@ static void write_elf_loads(DumpState *s, Error **errp)
 get_offset_range(memory_mapping->phys_addr,
  memory_mapping->length,
  s, &offset, &filesz);
-if (s->dump_info.d_class == ELFCLASS64) {
+if (dump_is_64bit(s)) {
 write_elf64_load(s, memory_mapping, phdr_index++, offset,
  filesz, errp);
 } else {
@@ -527,7 +532,7 @@ static void dump_begin(DumpState *s, Error **errp)
  */
 
 /* write elf header to vmcore */
-if (s->dump_info.d_class == ELFCLASS64) {
+if (dump_is_64bit(s)) {
 write_elf64_header(s, errp);
 } else {
 write_elf32_header(s, errp);
@@ -536,7 +541,7 @@ static void dump_begin(DumpState *s, Error **errp)
 return;
 }
 
-if (s->dump_info.d_class == ELFCLASS64) {
+if (dump_is_64bit(s)) {
 /* write PT_NOTE to vmcore */
 write_elf64_note(s, errp);
 if (*errp) {
@@ -747,7 +752,7 @@ static void get_note_sizes(DumpState *s, const void *note,
 uint64_t name_sz;
 uint64_t desc_sz;
 
-if (s->dump_info.d_class == ELFCLASS64) {
+if (dump_is_64bit(s)) {
 const Elf64_Nhdr *hdr = note;
 note_head_sz = sizeof(Elf64_Nhdr);
 name_sz = tswap64(hdr->n_namesz);
@@ -1007,7 +1012,7 @@ out:
 
 static void write_dump_header(DumpState *s, Error **errp)
 {
-if (s->dump_info.d_class == ELFCLASS32) {
+if (!dump_is_64bit(s)) {
 create_header32(s, errp);
 } else {
 create_header64(s, errp);
@@ -1697,7 +1702,7 @@ static void dump_init(DumpState *s, int fd, bool 
has_format,
 uint32_t size;
 uint16_t format;
 
-note_head_size = s->dump_info.d_class == ELFCLASS32 ?
+note_head_size = !dump_is_64bit(s) ?
 sizeof(Elf32_Nhdr) : sizeof(Elf64_Nhdr);
 
 format = le16_to_cpu(vmci->vmcoreinfo.guest_format);
@@ -1801,7 +1806,7 @@ static void dump_init(DumpState *s, int fd, bool 
has_format,
 }
 }
 
-if (s->dump_info.d_class == ELFCLASS64) {
+if (dump_is_64bit(s)) {
 s->phdr_offset = sizeof(Elf64_Ehdr);
 s->shdr_offset = s->phdr_offset + sizeof(Elf64_Phdr) * s->phdr_num;
 s->note_offset = s->shdr_offset + sizeof(Elf64_Shdr) * s->shdr_num;
-- 
2.32.0