Index: /trunk/src/VBox/Devices/Audio/DevIchAc97.cpp
===================================================================
--- /trunk/src/VBox/Devices/Audio/DevIchAc97.cpp	(revision 71740)
+++ /trunk/src/VBox/Devices/Audio/DevIchAc97.cpp	(revision 71741)
@@ -205,5 +205,6 @@
 };
 
-/** Emits registers for a specific (Native Audio Bus Master BAR) NABMBAR. */
+/** Emits registers for a specific (Native Audio Bus Master BAR) NABMBAR.
+ * @todo This totally messes with grepping for identifiers and tagging.  */
 #define AC97_NABMBAR_REGS(prefix, off)                                    \
     enum {                                                                \
@@ -1064,4 +1065,5 @@
 
 #ifdef VBOX_WITH_AUDIO_AC97_ASYNC_IO
+
 /**
  * Asynchronous I/O thread for an AC'97 stream.
@@ -1275,4 +1277,5 @@
     ASMAtomicXchgBool(&pAIO->fEnabled, fEnable);
 }
+
 #endif /* VBOX_WITH_AUDIO_AC97_ASYNC_IO */
 
@@ -1324,6 +1327,6 @@
         }
         else
-        {
 #endif
+        {
             const uint32_t cbSinkWritable = AudioMixerSinkGetWritable(pSink);
 
@@ -1349,7 +1352,5 @@
             AssertRC(rc2);
 
-#ifdef VBOX_WITH_AUDIO_AC97_ASYNC_IO
-        }
-#endif
+        }
     }
     else /* Input (SDI). */
@@ -1362,6 +1363,6 @@
         }
         else
-        {
 #endif
+        {
             rc2 = AudioMixerSinkUpdate(pSink);
             AssertRC(rc2);
@@ -1386,12 +1387,10 @@
                 AssertRC(rc2);
             }
-#ifdef VBOX_WITH_AUDIO_AC97_ASYNC_IO
-        }
-#endif
+        }
 
 #ifdef VBOX_WITH_AUDIO_AC97_ASYNC_IO
         if (fInTimer)
-        {
 #endif
+        {
             const uint32_t cbToTransfer = ichac97StreamGetUsed(pStream);
             if (cbToTransfer)
@@ -1402,7 +1401,5 @@
                 AssertRC(rc2);
             }
-#ifdef VBOX_WITH_AUDIO_AC97_ASYNC_IO
-        }
-#endif
+        }
     }
 }
@@ -1418,10 +1415,6 @@
 static void ichac97MixerSet(PAC97STATE pThis, uint8_t uMixerIdx, uint16_t uVal)
 {
-    if (size_t(uMixerIdx + 2) > sizeof(pThis->mixer_data))
-    {
-        AssertMsgFailed(("Index %RU8 out of bounds(%zu)\n", uMixerIdx, sizeof(pThis->mixer_data)));
-        return;
-    }
-
+    AssertMsgReturnVoid(uMixerIdx + 2U <= sizeof(pThis->mixer_data),
+                         ("Index %RU8 out of bounds (%zu)\n", uMixerIdx, sizeof(pThis->mixer_data)));
     pThis->mixer_data[uMixerIdx + 0] = RT_LO_U8(uVal);
     pThis->mixer_data[uMixerIdx + 1] = RT_HI_U8(uVal);
@@ -1437,15 +1430,8 @@
 static uint16_t ichac97MixerGet(PAC97STATE pThis, uint32_t uMixerIdx)
 {
-    uint16_t uVal;
-
-    if (size_t(uMixerIdx + 2) > sizeof(pThis->mixer_data))
-    {
-        AssertMsgFailed(("Index %RU8 out of bounds (%zu)\n", uMixerIdx, sizeof(pThis->mixer_data)));
-        uVal = UINT16_MAX;
-    }
-    else
-        uVal = RT_MAKE_U16(pThis->mixer_data[uMixerIdx + 0], pThis->mixer_data[uMixerIdx + 1]);
-
-    return uVal;
+    AssertMsgReturn(uMixerIdx + 2U <= sizeof(pThis->mixer_data),
+                         ("Index %RU8 out of bounds (%zu)\n", uMixerIdx, sizeof(pThis->mixer_data)),
+                    UINT16_MAX);
+    return RT_MAKE_U16(pThis->mixer_data[uMixerIdx + 0], pThis->mixer_data[uMixerIdx + 1]);
 }
 
@@ -1664,4 +1650,5 @@
     PPDMAUDIOSTREAMCFG pCfg     = &pStream->State.Cfg;
     PAUDMIXSINK        pMixSink = NULL;
+    AssertCompile(sizeof(pCfg->szName) >= 8);
 
     switch (pStream->u8SD)
@@ -1673,6 +1660,5 @@
             pCfg->DestSource.Source = PDMAUDIORECSOURCE_LINE;
             pCfg->enmLayout         = PDMAUDIOSTREAMLAYOUT_NON_INTERLEAVED;
-
-            RTStrPrintf2(pCfg->szName, sizeof(pCfg->szName), "Line-In");
+            strcpy(pCfg->szName, "Line-In");
 
             pMixSink                = pThis->pSinkLineIn;
@@ -1686,6 +1672,5 @@
             pCfg->DestSource.Source = PDMAUDIORECSOURCE_MIC;
             pCfg->enmLayout         = PDMAUDIOSTREAMLAYOUT_NON_INTERLEAVED;
-
-            RTStrPrintf2(pCfg->szName, sizeof(pCfg->szName), "Mic-In");
+            strcpy(pCfg->szName, "Mic-In");
 
             pMixSink                = pThis->pSinkMicIn;
@@ -1699,6 +1684,5 @@
             pCfg->DestSource.Dest   = PDMAUDIOPLAYBACKDEST_FRONT;
             pCfg->enmLayout         = PDMAUDIOSTREAMLAYOUT_NON_INTERLEAVED;
-
-            RTStrPrintf2(pCfg->szName, sizeof(pCfg->szName), "Output");
+            strcpy(pCfg->szName, "Output");
 
             pMixSink                = pThis->pSinkOut;
@@ -1990,8 +1974,7 @@
         case AC97SOUNDSOURCE_MC_INDEX: return &pThis->StreamMicIn;
         case AC97SOUNDSOURCE_PO_INDEX: return &pThis->StreamOut;
-        default:                       break;
-    }
-
-    return NULL;
+        default:                       return NULL;
+    }
+
 }
 
@@ -2127,4 +2110,5 @@
 
 #ifndef VBOX_WITH_AUDIO_AC97_CALLBACKS
+
 /**
  * Starts the internal audio device timer.
@@ -2297,4 +2281,5 @@
     ichac97TimerMain(pThis);
 }
+
 #endif /* !VBOX_WITH_AUDIO_AC97_CALLBACKS */
 
@@ -2447,4 +2432,6 @@
                 if (cbSrc)
                 {
+/** @todo r=bird: Just curious, DevHDA uses PDMDevHlpPCIPhysWrite here.  So,
+ *        is AC97 not subject to PCI busmaster enable/disable? */
                     int rc2 = PDMDevHlpPhysWrite(pThis->CTX_SUFF(pDevIns), pRegs->bd.addr, (uint8_t *)pvSrc, cbSrc);
                     AssertRC(rc2);
@@ -2919,9 +2906,10 @@
     PAC97STATE pThis = (PAC97STATE)pvUser;
 
-    DEVAC97_LOCK(pThis);
+    DEVAC97_LOCK(pThis); /** @todo r=bird: some disconnect between the remark of this function docs and this locking statement.  */
 
     int rc = VINF_SUCCESS;
 
     uint32_t index = uPort - pThis->IOPortBase[0];
+    Assert(index < 256);
 
     switch (cbVal)
@@ -2937,13 +2925,6 @@
         case 2:
         {
-            *pu32Val = UINT32_MAX;
             pThis->cas = 0;
-
-            switch (index)
-            {
-                default:
-                    *pu32Val = ichac97MixerGet(pThis, index);
-                    break;
-            }
+            *pu32Val = ichac97MixerGet(pThis, index);
             break;
         }
@@ -2981,6 +2962,5 @@
  * @remarks Caller enters the device critical section.
  */
-static DECLCALLBACK(int) ichac97IOPortNAMWrite(PPDMDEVINS pDevIns, void *pvUser, RTIOPORT uPort,
-                                               uint32_t u32Val, unsigned cbVal)
+static DECLCALLBACK(int) ichac97IOPortNAMWrite(PPDMDEVINS pDevIns, void *pvUser, RTIOPORT uPort, uint32_t u32Val, unsigned cbVal)
 {
     RT_NOREF(pDevIns);
@@ -3410,27 +3390,4 @@
 
 /**
- * @interface_method_impl{PDMDEVREG,pfnDestruct}
- */
-static DECLCALLBACK(int) ichac97Destruct(PPDMDEVINS pDevIns)
-{
-    PAC97STATE pThis = PDMINS_2_DATA(pDevIns, PAC97STATE);
-
-    LogFlowFuncEnter();
-
-    PAC97DRIVER pDrv, pDrvNext;
-    RTListForEachSafe(&pThis->lstDrv, pDrv, pDrvNext, AC97DRIVER, Node)
-    {
-        RTListNodeRemove(&pDrv->Node);
-        RTMemFree(pDrv);
-    }
-
-    /* Sanity. */
-    Assert(RTListIsEmpty(&pThis->lstDrv));
-
-    return VINF_SUCCESS;
-}
-
-
-/**
  * Attach command, internal version.
  *
@@ -3665,16 +3622,40 @@
 
 /**
+ * @interface_method_impl{PDMDEVREG,pfnDestruct}
+ */
+static DECLCALLBACK(int) ichac97Destruct(PPDMDEVINS pDevIns)
+{
+    PDMDEV_CHECK_VERSIONS_RETURN_QUIET(pDevIns); /* this shall come first */
+    PAC97STATE pThis = PDMINS_2_DATA(pDevIns, PAC97STATE);
+
+    LogFlowFuncEnter();
+
+    PAC97DRIVER pDrv, pDrvNext;
+    RTListForEachSafe(&pThis->lstDrv, pDrv, pDrvNext, AC97DRIVER, Node)
+    {
+        RTListNodeRemove(&pDrv->Node);
+        RTMemFree(pDrv);
+    }
+
+    /* Sanity. */
+    Assert(RTListIsEmpty(&pThis->lstDrv));
+
+    return VINF_SUCCESS;
+}
+
+/**
  * @interface_method_impl{PDMDEVREG,pfnConstruct}
  */
 static DECLCALLBACK(int) ichac97Construct(PPDMDEVINS pDevIns, int iInstance, PCFGMNODE pCfg)
 {
-    RT_NOREF(iInstance);
+    PDMDEV_CHECK_VERSIONS_RETURN(pDevIns); /* this shall come first */
     PAC97STATE pThis = PDMINS_2_DATA(pDevIns, PAC97STATE);
-
-    /* NB: This must be done *before* any possible failure (and running the destructor). */
+    Assert(iInstance == 0); RT_NOREF(iInstance);
+
+    /*
+     * Initialize data so we can run the destructor without scewing up.
+     */
     RTListInit(&pThis->lstDrv);
 
-    Assert(iInstance == 0);
-    PDMDEV_CHECK_VERSIONS_RETURN(pDevIns);
 
     /*
