On 8/21/17 2:08 PM, Roger Riggs wrote:
Please review a rmi testlibrary enhancement to allow a trigger byte sequence 
before
the match and replace. It improves the ease with which stream contents can be
identified
when streams contain IP addresses and sequence numbers that change from run to 
run.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-socketfactory-trigger-8186539/

Hi Roger,

Overall looks fine, just a couple nitpicky comments.

There are two definitions of EMPTY_BYTE_ARRAY; one in the TestSocketFactory class and the second in the nested class MatchReplaceOutputStream. It doesn't really matter, though I did find it a bit confusing.

233 * The trigger, match, and replacements are propagated to all existing sockets.

This is in InterposeSocket, which has no sockets to propagate this state to (unlike the other cases). Maybe this means that this state is propagated to any existing OutputStream.

It would be helpful to have a brief comment that describes exactly how the triggering mechanism. I think that if triggerBytes has nonzero length, then the MatchReplaceOutputStream waits until it sees the triggerBytes, after which one match/replace is enabled; further match/replace operations don't occur until after triggerBytes is seen again. Also, if triggerBytes is zero length, then match/replace is always done. Is that correct?

This is mostly just changes to comments, so I don't think I need to see another webrev.

s'marks

Reply via email to