Re: [13] RFR: JDK-8219914: Change the environment variable for Java Access Bridge logging to have a directory.

[Phil Race]

Hi,

It is still inconsistent. the "+1" and "+5" logic relies on the answer 
to sizeof(char) being 1.
So either get rid of all those calls or use sizeof('/') and 
sizeof(".logX"); instead of 1 and 5.

I'd get rid of them ....

-phil

On 3/27/19 1:41 AM, Krishna Addepalli wrote:

Hi Phil,

Thanks for the review.

I have changed the variable to “JAVA_ACCESSBRIDGE_LOGDIR”.

Yes, ‘/’ file separator character works on windows. I have used it in 
the past and have also currently tested it on my machine and it works.

I have added the multiplier “sizeof(char)” for all memcpy and memset 
lines in the code, to keep it consistent. Thanks for pointing that out.

Here is the link to the webrev: 
http://cr.openjdk.java.net/~kaddepalli/8219914/webrev01/

Thanks,

Krishna

*From:*Phil Race
*Sent:* Tuesday, March 26, 2019 10:42 PM
*To:* Krishna Addepalli <krishna.addepa...@oracle.com>; 
swing-dev@openjdk.java.net
*Subject:* Re: <Swing Dev> [13] RFR: JDK-8219914: Change the 
environment variable for Java Access Bridge logging to have a directory.

Can we just call it  JAVA_ACCESSBRIDGE_LOGDIR ?
 filePath[envFilePathLength] = '/';
Is this right ? Does fopen on Windows expect this unix style separator ?
53         memcpy(filePath, envfilePath, envFilePathLength*sizeof(char));
56         memcpy(filePath + envFilePathLength + 1 + fileNameLength, 
".log", 4*sizeof(char));
Interesting that you feel it necessary to use sizeof(char) when 
clearly the whole logic, eg see :
50         auto filePathSize = envFilePathLength + 1 + fileNameLength 
+ 5; //1 for "/", 5 for ".log" and 0;
assumes it is 1 ...
  PrintDebugString("couldnot open file %s", filePath);

couldnot -> could not

-phil.


On 3/26/19 2:45 AM, Krishna Addepalli wrote:

    Hi Phil,

    Per our discussion, I have changed the JAVA_ACCESSBRIDGE_LOGFILE
    to JAVA_ACCESSBRIDGE_LOGDIRECTORY to reflect that it accepts only
    a directory value in the variable.

    I have also changed the code in AccessBridgeDebug.cpp appropriately.

    So, currently, the code will look for the environment variable,
    which should contain path to the directory, and two log files
    namely “java_access_bridge.log” and “windows_access_bridge.log”
    will be created.

    Link to the JDK Issue:
    https://bugs.openjdk.java.net/browse/JDK-8219914

    Here is the webrev:
    http://cr.openjdk.java.net/~kaddepalli/8219914/webrev00/

    Thanks,

    Krishna