Kathey Marsden wrote:
Attached is the updated patch for Network Server XAMGR Level 7 support.

[ snip ]

+ {
+ connThread.trace("syncType = " + syncTypeToString(syncType));
+ connThread.trace("xid = " + xid);
+ connThread.trace("xaflags =" + xaflagsToString(xaflags));
+ }
+
+ switch (syncType)
+ { + case CodePoint.SYNCTYPE_NEW_UOW:
+ // new unit of work for XA
+ // formatId -1 is just a local connection
+ startXATransaction(xid,xaflags);
+ break;

[ snip ]

+                       default:
+                               connThread.invalidCodePoint(codePoint);
+               }
+               if (syncType != CodePoint.SYNCTYPE_INDOUBT)
+               {
+                       if (xid == null)
+                               connThread.missingCodePoint(CodePoint.XID);
+                       
+                       if (! readXAFlags)
+                               if (SanityManager.DEBUG)
+                                       
connThread.missingCodePoint(CodePoint.XAFLAGS);
+               }
+       }

One Comment:

Is there a reason why we have the check for missingCodePoint AFTER the "switch", instead of before? Since "xid" and "xaFlags" are both set by the time we get to the switch (as is "syncType"), and since both are potentially used within the switch statement itself, it seems like we should be checking for their existence _before_ we enter the switch. That way, we can throw an error before trying to do any of the work...

Does that sound right?

Otherwise, changes look good.

+1

Army

PS As for testing: I'm sure it goes without saying, but what the heck, might as well get it in print--have you run the derbynet and other related suites/tests with these changes, to make sure there aren't any regressions?

Reply via email to