Thanks for the review Shashi. Sergey could you also review?
Thanks, Krishna > On 05-Dec-2018, at 10:16 AM, Shashidhara Veerabhadraiah > <shashidhara.veerabhadra...@oracle.com> wrote: > > Hi Krishna, The changes looks good to me and I did imported this patch and > found that the debug logs are useful in debugging the accessibility related > issues. I could see the actions being done on the java program reflected in > the corresponding java and windows native logs. And thanks for adding the > classification for the logs. > > Thanks and regards, > Shashi > > -----Original Message----- > From: Krishna Addepalli > Sent: Tuesday, December 4, 2018 4:44 PM > To: Shashidhara Veerabhadraiah <shashidhara.veerabhadra...@oracle.com>; > Sergey Bylokhov <sergey.bylok...@oracle.com>; Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com>; awt-dev@openjdk.java.net > Subject: RE: <AWT Dev> RFR: [12] JDK-8196681: Java Access Bridge logging and > debug flags dynamically controlled > > Hi Shashi, > > Thanks for the comments, here is the updated webrev: > http://cr.openjdk.java.net/~kaddepalli/8196681/webrev05 > > Krishna > > -----Original Message----- > From: Shashidhara Veerabhadraiah > Sent: Tuesday, December 4, 2018 2:29 PM > To: Krishna Addepalli <krishna.addepa...@oracle.com>; Sergey Bylokhov > <sergey.bylok...@oracle.com>; Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com>; awt-dev@openjdk.java.net > Subject: RE: <AWT Dev> RFR: [12] JDK-8196681: Java Access Bridge logging and > debug flags dynamically controlled > > Hi Krishna, Environment variable set to a file name(with an extension), did > created a file with that extension. But without it, the extensions were > blank. So I suggest to default to .log if there is no extension set by the > user(then the system can choose) as it is a log of debug prints and the file > can be opened instantly. > > Thanks and regards, > Shashi > > -----Original Message----- > From: Shashidhara Veerabhadraiah > Sent: Monday, December 3, 2018 10:46 PM > To: Krishna Addepalli <krishna.addepa...@oracle.com>; Sergey Bylokhov > <sergey.bylok...@oracle.com>; Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com>; awt-dev@openjdk.java.net > Subject: Re: <AWT Dev> RFR: [12] JDK-8196681: Java Access Bridge logging and > debug flags dynamically controlled > > Hi Krishna, Sure I will check it once again tomorrow for the logger file > extension as I am not able to get the JAWS license from home network. > > Another problem that I see is that the way we use the environment variable. > We need to set the flag pointing to a single text file it will end up in > creating 2 text files from 2 different sources but both the names does not > match the environment variable value. So it is good to separate them out or > you can clearly mention the notes in detail. > > Thanks and regards, > Shashi > > -----Original Message----- > From: Krishna Addepalli > Sent: Monday, December 3, 2018 8:05 PM > To: Shashidhara Veerabhadraiah <shashidhara.veerabhadra...@oracle.com>; > Sergey Bylokhov <sergey.bylok...@oracle.com>; Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com>; awt-dev@openjdk.java.net > Subject: RE: <AWT Dev> RFR: [12] JDK-8196681: Java Access Bridge logging and > debug flags dynamically controlled > > Hi Shashi, > > 1. It is strange that the Windows Log file did not have any extension and the > Java log file had an extension. The code for reading the environment variable > content and creating the appropriate file is common between > JavaAccessBridge.dll and WindowsAccessBridge.dll, so if one is able to read > the environment variable with the extension, so should the other. > The scenario where I see this could be broken could be that, you update the > environment variable, and not restart either the JAWs application or the > Cygwin/Java IDE from which you run the java program. > > 2. Since this is a debug feature, it is expected that the developers are > aware of the log file getting written out from JAWs side as well > (WindowsAccessBridge.dll). > Nevertheless, I agree with your point that if the JAWs is active for long > time, the log file can get pretty big. > But, even if we add another environment variable, the problem remains the > same. So, I think we can only add a note saying that it is recommended to > close JAWs after each debug session. > > Thanks, > Krishna > > -----Original Message----- > From: Shashidhara Veerabhadraiah > Sent: Monday, December 3, 2018 10:44 AM > To: Krishna Addepalli <krishna.addepa...@oracle.com>; Sergey Bylokhov > <sergey.bylok...@oracle.com>; Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com>; awt-dev@openjdk.java.net > Subject: RE: <AWT Dev> RFR: [12] JDK-8196681: Java Access Bridge logging and > debug flags dynamically controlled > > Hi Krishna, I have imported the patch and used it for some testing. The debug > logs helped with debug information of the actions that I performed with a > demo application. Below are few suggestions: > > 1. The Windows log file did not had any extensions whereas the java log had > the file extension. May be a default extension could be built in. > 2. Another thing I felt was that the windows log file was populated by the > access bridge module causing it to write it for any version of java usage and > whereas Java log file will be written for the built java module. Since both > of them are behaving differently can we have 2 separate variables for it > instead of one? Since the windows log file can grow to larger size and for > debugging java log is sufficient at times, we can keep it separate. > > Otherwise the changes looks good to me. > > Thanks and regards, > Shashi > > -----Original Message----- > From: Krishna Addepalli > Sent: Wednesday, October 17, 2018 1:04 AM > To: Sergey Bylokhov <sergey.bylok...@oracle.com>; Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com>; awt-dev@openjdk.java.net > Subject: Re: <AWT Dev> RFR: [12] JDK-8196681: Java Access Bridge logging and > debug flags dynamically controlled > > Hi Sergey, > > Per our conversation, I have made the following changes: > 1. Add logging support to WindowsAccessBridge as well. > 2. Added timestamp info in the log file. > 3. Added INFO/WARN/ERROR tags as appropriate in all the places where the > logging function is called. > 4. JavaAccessBridge will generate the file suffixed with > "_java_access_bridge", whereas WindowsAccessBridge generates file suffixed > with "_windows_access_bridge". > > Here is the updated webrev: > http://cr.openjdk.java.net/~kaddepalli/8196681/webrev04/ > > Thanks, > Krishna > > -----Original Message----- > From: Sergey Bylokhov > Sent: Tuesday, September 18, 2018 3:26 AM > To: Prasanta Sadhukhan <prasanta.sadhuk...@oracle.com>; Krishna Addepalli > <krishna.addepa...@oracle.com>; awt-dev@openjdk.java.net > Subject: Re: <AWT Dev> RFR: [12] JDK-8196681: Java Access Bridge logging and > debug flags dynamically controlled > > I guess that it is a good thing to use nullptr, but I think it is better to > follow the style used in the file, and replace NULL to nullptr in the whole > file in some separate fix. > > On 14/09/2018 05:18, Prasanta Sadhukhan wrote: >> one question: why nullptr is used? I see we used NULL in other places >> in this file, why cant we use the same? >> >> Regards >> Prasanta >> On 14-Sep-18 4:05 PM, Krishna Addepalli wrote: >>> >>> Thanks for the comments. Here is the new webrev: >>> http://cr.openjdk.java.net/~kaddepalli/8196681/webrev02/ >>> <http://cr.openjdk.java.net/%7Ekaddepalli/8196681/webrev02/> >>> >>> --Krishna >>> >>> *From:*Prasanta Sadhukhan >>> *Sent:* Friday, September 14, 2018 3:23 PM >>> *To:* Krishna Addepalli <krishna.addepa...@oracle.com>; >>> awt-dev@openjdk.java.net >>> *Subject:* Re: <AWT Dev> RFR: [12] JDK-8196681: Java Access Bridge >>> logging and debug flags dynamically controlled >>> >>> ok. The formatting is screwed up, needs to be rectified. Also l194, >>> there should be a space before { >>> >>> Regards >>> Prasantaa >>> >>> On 14-Sep-18 2:45 PM, Krishna Addepalli wrote: >>> >>> Thanks for the review Prasanta. Although there is not much >>> difference between using fprintf and vfprintf, I have changed all >>> the calls to vfprintf. >>> >>> Here is the new webrev: >>> http://cr.openjdk.java.net/~kaddepalli/8196681/webrev01/ >>> <http://cr.openjdk.java.net/%7Ekaddepalli/8196681/webrev01/> >>> >>> Krishna >>> >>> *From:*Prasanta Sadhukhan >>> *Sent:* Friday, September 14, 2018 11:38 AM >>> *To:* Krishna Addepalli <krishna.addepa...@oracle.com> >>> <mailto:krishna.addepa...@oracle.com>; awt-dev@openjdk.java.net >>> <mailto:awt-dev@openjdk.java.net> >>> *Subject:* Re: <AWT Dev> RFR: [12] JDK-8196681: Java Access Bridge >>> logging and debug flags dynamically controlled >>> >>> One thing, since you are passing va_list as the last parameter, >>> shouldn't all fprintf be actually vfprintf? >>> >>> Regards >>> Prasanta >>> >>> On 14-Sep-18 12:55 AM, Krishna Addepalli wrote: >>> >>> Hi All, >>> >>> Enhancement: https://bugs.openjdk.java.net/browse/JDK-8196681 >>> >>> Webrev: >>> http://cr.openjdk.java.net/~kaddepalli/8196681/webrev00/ >>> <http://cr.openjdk.java.net/%7Ekaddepalli/8196681/webrev00/> >>> >>> Please review an enhancement for supporting the logging of >>> Java Access Bridge, so that customers/users facing problems >>> with Accessibility can turn on/off logging to debug problems >>> with this feature. >>> >>> The proposed solution is to have the user/developer define an >>> environment variable "*JAVA_ACCESSBRIDGE_LOGFILE*" and filling >>> with the path to the log file. >>> >>> The JavaAccessBridge will read the variable, and write to the >>> file in the path provided. >>> Note that the implementation is simplistic, and doesnot have >>> any way to handle multiple applications, since it is a debug >>> only feature. >>> >>> Thanks, >>> >>> Krishna >>> >> > > > -- > Best regards, Sergey.
