Index: /trunk/src/VBox/Devices/Audio/DevCodec.cpp
===================================================================
--- /trunk/src/VBox/Devices/Audio/DevCodec.cpp	(revision 31214)
+++ /trunk/src/VBox/Devices/Audio/DevCodec.cpp	(revision 31215)
@@ -155,4 +155,5 @@
 {
     uint32_t *pu32Bparam = NULL;
+    //** @todo r=michaln: Missing NID bounds checking!
     PCODECNODE pNode = &pState->pNodes[CODEC_NID(cmd)];
     *pResp = 0;
@@ -184,4 +185,5 @@
 {
     Assert((CODEC_CAD(cmd) == pState->id));
+    //** @todo r=michaln: This is not bounds checked and may overflow the pNodes array!
     *pResp = pState->pNodes[CODEC_NID(cmd)].node.au32F00_param[cmd & CODEC_VERB_8BIT_DATA];
     return VINF_SUCCESS;
@@ -195,5 +197,7 @@
         *pResp = pState->pNodes[CODEC_NID(cmd)].adcmux.u32F01_param;
     else if (STAC9220_IS_DIGOUTPIN_CMD(cmd))
+        //** @todo r=michaln: Is that really u32F07, or should it be u32F01?
         *pResp = pState->pNodes[CODEC_NID(cmd)].digout.u32F07_param;
+    //** @todo r=michaln: Else what? We must always fill out *pResp!
     return VINF_SUCCESS;
 }
@@ -228,4 +232,5 @@
         *pResp = pState->pNodes[CODEC_NID(cmd)].cdnode.u32F07_param;
     else
+        //** @todo r=michaln: pResp must still be filled out
         AssertMsgFailed(("Unsupported"));
     return VINF_SUCCESS;
@@ -284,4 +289,5 @@
         pu32Reg = &pState->pNodes[CODEC_NID(cmd)].volumeKnob.u32F08_param;
     else
+        //** @todo r=michaln: This will crash in release builds! (pu32Reg will be NULL here)
         AssertMsgFailed(("unsupported operation %x on node: %x\n", CODEC_VERB_CMD8(cmd), CODEC_NID(cmd)));
     Assert(pu32Reg);
@@ -324,4 +330,5 @@
 {
     Assert((CODEC_CAD(cmd) == pState->id));
+    //** @todo r=michaln: Again, possible pNodes array overflow
     *pResp = *(uint32_t *)&pState->pNodes[CODEC_NID(cmd)].node.au8F02_param[cmd & CODEC_VERB_8BIT_DATA];
     return VINF_SUCCESS;
@@ -389,4 +396,5 @@
         *pResp = pState->pNodes[CODEC_NID(cmd)].afg.u32F20_param;
     }
+    //** @todo r=michaln: pResp must always be filled out
     return VINF_SUCCESS;
 }
@@ -458,4 +466,5 @@
     else
         *pResp = 0; /* STAC9220 6.20 6.13-6.18: no response supposed for this verb. */
+    //** @todo r=michaln: Is this intentional? I don't think we should always return 0?
     *pResp = 0;
     return VINF_SUCCESS;
@@ -525,4 +534,5 @@
     else if (STAC9220_IS_DIGINPIN_CMD(cmd))
         *pResp = pState->pNodes[CODEC_NID(cmd)].digin.u32F0c_param;
+    //** @todo r=michaln: Do we really always want to return zero?
     *pResp = 0;
     return VINF_SUCCESS;