Codereview request for 7171184: Make the failure message in 7168110 more specific

[David Holmes]

On 24/05/2012 7:02 AM, Alan Bateman wrote:
> On 23/05/2012 20:37, Rob McKenna wrote:
>> Hi Alan,
>>
>> Sorry, you've picked me up on that "if (" space a number of times.
>> Thanks for pointing it out again.
>>
>> What should I be checking w.r.t. the ProcessIdToSessionId? It returns
>> a 0 on failure so once the session id's are unequal or we've
>> encountered an ERROR_ACCESS_DENIED we should really be good to go right?
>>
>> Looking at that reminds me that I had got an && instead of an || for
>> the session id / error check. Its a good point that if the
>> ProcessIdToSessionId call fails for a reason outside ACCESS_DENIED, we
>> won't know about it. I'm wondering if I should revert that now?
> What you have will work but we shouldn't be comparing pSid to cSid
> unless both calls to ProcessIdToSessionId succeed.

Yeah I raised this previously with Rob and the response was that on 
failure the sessionId will be set to zero but I do not see that in the 
docs for ProcessIdToSessionId. The other assumptions was that requesting 
the session Id for the current process could never fail - I don't know 
how valid that is (these APIs don't document any actual errors). If 
those two assumptions hold then the current code is correct, otherwise 
... something like this:

if (( ProcessIdToSessionId( GetCurrentProcessId(), &cSid) != 0) {
     BOOL result = ProcessIdToSessionId(GetProcessId(hProcess), &pSid);
     if (result != 0) { // have valid sessionIds
         if  (pSid != cSid)
             JNU_ThrowIOException(env, "useful message");
         // else same session so original memory error stands
     } else if (GetLastError() == ERROR_ACCESS_DENIED) {
         // implies that the sessions must be different
         JNU_ThrowIOException(env, "useful message");
      }
      // else some other error getting other session id
}
// else some error getting current session Id

But this needs to ensure the original error is not lost, and it would be 
nice not to have two throws statements :(

I get the feeling what seemed like a simple suggestion is getting out of 
control. Feel free to drop this.

David
-----


> -Alan.