Re: [PATCH 3/3] hw/pci-host/q35.c: Avoid early return in mch_write_config()

[Michael Tokarev]

On 7/8/26 15:10, Peter Maydell wrote:

In mch_write_config() we return early if has_smm_ranges is false.
This is slightly bug-prone because it leaves the door open to somebody
later adding non-SMM-specific code at the bottom of the function.

This case isn't as bad as the one in realize, because the function is
a lot shorter.  But putting the handling of the three SMM specific
ranges into an if() rather than having an early return seems better.

Yes, this is a good small refactoring, - at the time I haven't seen
the same pattern in another place.

Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>

Reviewed-by: Michael Tokarev <m...@tls.msk.ru>

Thanks!

/mjt

  hw/pci-host/q35.c | 26 ++++++++++++--------------
  1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index e42b84b978..f4556ad03a 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -485,22 +485,20 @@ static void mch_write_config(PCIDevice *d,
          mch_update_pciexbar(mch);
      }

- if (!mch->has_smm_ranges) {

-        return;
-    }
+    if (mch->has_smm_ranges) {
+        if (ranges_overlap(address, len, MCH_HOST_BRIDGE_SMRAM,
+                           MCH_HOST_BRIDGE_SMRAM_SIZE)) {
+            mch_update_smram(mch);
+        }

- if (ranges_overlap(address, len, MCH_HOST_BRIDGE_SMRAM,

-                       MCH_HOST_BRIDGE_SMRAM_SIZE)) {
-        mch_update_smram(mch);
-    }
+        if (ranges_overlap(address, len, MCH_HOST_BRIDGE_EXT_TSEG_MBYTES,
+                           MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_SIZE)) {
+            mch_update_ext_tseg_mbytes(mch);
+        }

- if (ranges_overlap(address, len, MCH_HOST_BRIDGE_EXT_TSEG_MBYTES,

-                       MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_SIZE)) {
-        mch_update_ext_tseg_mbytes(mch);
-    }
-
-    if (ranges_overlap(address, len, MCH_HOST_BRIDGE_F_SMBASE, 1)) {
-        mch_update_smbase_smram(mch);
+        if (ranges_overlap(address, len, MCH_HOST_BRIDGE_F_SMBASE, 1)) {
+            mch_update_smbase_smram(mch);
+        }
      }
  }