qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 05/20] disas/nanomips: Remove __cond methods from class


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH 05/20] disas/nanomips: Remove __cond methods from class
Date: Tue, 16 Aug 2022 02:14:13 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.12.0

On 15/8/22 16:20, Thomas Huth wrote:
On 15/08/2022 13.07, Philippe Mathieu-Daudé wrote:
On 15/8/22 09:26, Milica Lazarevic wrote:
NMD class methods with the conditional_function type like
NMD::ADDIU_32__cond, NMD::ADDIU_RS5__cond, etc. are removed from the NMD
class. They're now declared global static functions. Therefore, typedef
of the function pointer, conditional_function is defined outside of the
class.

Now that conditional_function type functions are not part of the NMD
class we can't access them using the this pointer. Thus, the use of
the this pointer has been deleted.

Signed-off-by: Milica Lazarevic <milica.lazarevic@syrmia.com>
---
  disas/nanomips.cpp | 42 +++++++++++++++++++++---------------------
  disas/nanomips.h   | 14 ++------------
  2 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 039c353d0b..9e720d0e8d 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
@@ -787,7 +787,7 @@ int NMD::Disassemble(const uint16 * data, std::string & dis,
              if ((op_code & table[i].mask) == table[i].value) {
                  /* possible match */
                  conditional_function cond = table[i].condition;
-                if ((cond == 0) || (this->*cond)(op_code)) {
+                if ((cond == 0) || (cond)(op_code)) {

QEMU C style is more like this:

                    if ((cond == NULL) || cond(op_code)) {

                      try
                      {
                          if (table[i].type == pool) {

diff --git a/disas/nanomips.h b/disas/nanomips.h
index a795ed44e8..0e6670adf5 100644
--- a/disas/nanomips.h
+++ b/disas/nanomips.h
@@ -31,6 +31,8 @@ typedef uint32_t uint32;
  typedef uint16_t uint16;
  typedef uint64_t img_address;
+typedef bool(*conditional_function)(uint64 instruction);

Please add a space before the returned type. I'd rather
prefix functions extracted from the NMD class with `nmd_`:

But adding a prefix will also increase the size of the patches quite a bit (well, maybe not for this identifier here, but certainly for some other spots), so I think it's fair to keep it without prefix here, too. Just my 0.02 €.

The typedef is moved from the header to the source file in patch #9,
so OK.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]