Re: RFR: 8287766: Unnecessary Vector usage in LdapClient

2022-06-03 Thread Vyom Tewari
On Sun, 29 May 2022 20:22:55 GMT, Andrey Turbanov wrote: > In [JDK-8273098](https://bugs.openjdk.java.net/browse/JDK-8273098) I missed > one place, where Vector could be replaced with ArrayList. > Usage of thread-safe collection `Vector` is unnecessary. It's recommended to > use `ArrayList` if

Re: RFR: 8287544: Replace uses of StringBuffer with StringBuilder in java.naming

2022-05-31 Thread Vyom Tewari
On Sun, 29 May 2022 21:57:46 GMT, Andrey Turbanov wrote: > StringBuffer is a legacy synchronized class. StringBuilder is a direct > replacement to StringBuffer which generally have better performance. > There are some code that still uses StringBuffer in java.naming which could > be migrated to

Re: RFR: 8287497: Use String.contains() instead of String.indexOf() in java.naming

2022-05-31 Thread Vyom Tewari
On Sun, 29 May 2022 14:00:20 GMT, Andrey Turbanov wrote: > `String.contains` was introduced in Java 5. > Some code in java.naming still uses old approach with `String.indexOf` to > check if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to

Re: RFR: 8283411: InflaterInputStream holds on to a temporary byte array of 512 bytes [v2]

2022-03-20 Thread Vyom Tewari
On Sun, 20 Mar 2022 14:05:58 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which handles >> https://bugs.openjdk.java.net/browse/JDK-8283411? >> >> The commit here moves the temporary byte array from being a member of the >> class to a local variable within the `skip` me

Re: RFR: 8276042: Remove unused local variables in java.naming [v2]

2021-10-27 Thread Vyom Tewari
On Wed, 27 Oct 2021 15:42:32 GMT, Andrey Turbanov wrote: >> 8276042: Remove unused local variables in java.naming > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8276042: Remove unused local variables in java.naming >

Re: RFR: 8274949: Use String.contains() instead of String.indexOf() in java.base

2021-10-08 Thread Vyom Tewari
On Fri, 17 Sep 2021 08:56:47 GMT, Andrey Turbanov wrote: > String.contains was introduced in Java 5. > Some code in java.base still uses old approach with `String.indexOf` to check > if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to rea

Re: RFR: 8274464: Remove redundant stream() call before forEach in java.* modules

2021-09-29 Thread Vyom Tewari
On Wed, 15 Sep 2021 07:12:25 GMT, Andrey Turbanov wrote: > 8274464: Remove redundant stream() call before forEach in java.* modules Looks good to me. - Marked as reviewed by vtewari (Committer). PR: https://git.openjdk.java.net/jdk/pull/5520

Re: [jdk17] RFR: 8269772: [macos-aarch64] test compilation failed with "SocketException: No buffer space available"

2021-07-06 Thread Vyom Tewari
On Mon, 5 Jul 2021 11:21:39 GMT, Daniel Fuchs wrote: > Please find here a trivial fix for: > > 8269772: [macos-aarch64] test compilation failed with "SocketException: No > buffer space available" > > Running several of the websocket tests concurrently on some of the CI > machines is causing r

Re: RFR: 8269124: Update java.time to use switch expressions (part II)

2021-06-22 Thread Vyom Tewari
On Tue, 22 Jun 2021 10:50:17 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review the second half of my update for the `java.time` > package to make use of switch expressions? > > This PR was split into two parts due to the large number of files affected. > > Kind regards, >

Re: RFR: 8268113: Re-use Long.hashCode() where possible [v3]

2021-06-02 Thread Vyom Tewari
On Wed, 2 Jun 2021 16:37:49 GMT, Сергей Цыпанов wrote: >> There is a few JDK classes duplicating the contents of Long.hashCode() for >> hash code calculation. They should explicitly delegate to Long.hashCode(). > > Сергей Цыпанов has updated the pull request incrementally with one additional >

Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v9]

2021-05-31 Thread Vyom Tewari
On Mon, 31 May 2021 14:10:50 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.io`, >> `java.math`, and `java.text` packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated

Integrated: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
On Mon, 10 May 2021 06:34:36 GMT, Vyom Tewari wrote: > this change will include the below headers files to Linux only. > #include > #include This pull request has now been integrated. Changeset: b823b3ef Author:Vyom Tewari URL: https://git.openjdk.java.net/j

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
On Mon, 10 May 2021 09:50:44 GMT, Aleksey Shipilev wrote: >> this change will include the below headers files to Linux only. >> #include >> #include > > src/java.base/unix/native/libjava/io_util_md.c line 39: > >> 37: #if defined(__linux__) >> 38: #include >> 39: #include > > Does Mac OS

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
On Mon, 10 May 2021 11:26:30 GMT, Alan Bateman wrote: >> this change will include the below headers files to Linux only. >> #include >> #include > > src/java.base/unix/native/libjava/io_util_md.c line 256: > >> 254: return -1; >> 255: } >> 256: #if defined(__linux__) && defi

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
On Mon, 10 May 2021 11:09:07 GMT, Vyom Tewari wrote: >> I am not the Mac expert, but i checked on my MacOS Laptop and i did not >> found "BLKGETSIZE64" defined in sys/stat.h. >> As Alan already told that i took the code from FileDispatcherImpl.size0 and >>

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
On Mon, 10 May 2021 10:59:37 GMT, Vyom Tewari wrote: >> I think Vymon took the code from FileDispatcherImpl_size0, which has been >> compiling on macOS without issues. I don't mind if we fix the issue with >> this PR or just back it out and fix it with another PR. >

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
On Mon, 10 May 2021 10:44:10 GMT, Alan Bateman wrote: >> OK, so what you are saying is that the path that references `S_ISBLK` is >> protected by `BLKGETSIZE64` that is guaranteed to be undefined on OS X? If >> so, this is (awkwardly) fine. > > I think Vymon took the code from FileDispatcherImp

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
On Mon, 10 May 2021 11:21:34 GMT, Vyom Tewari wrote: >>> Hi Vyom, >>> >>> That fixes the include file problem but as this was obviously not tested on >>> non-Linux have you checked that the rest of the fix for 8266610 works okay >>> on non-Linu

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
On Mon, 10 May 2021 06:34:36 GMT, Vyom Tewari wrote: > this change will include the below headers files to Linux only. > #include > #include somehow tests not working for me. https://github.com/vyommani/jdk/actions/runs/827989185 - PR: https://git.openjdk.java.net

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
On Mon, 10 May 2021 13:01:45 GMT, Jayathirth D V wrote: > In internal CI with fix tier 1 builds are running fine. Ok i will integrate my changes now. - PR: https://git.openjdk.java.net/jdk/pull/3943

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
On Mon, 10 May 2021 08:46:15 GMT, Vyom Tewari wrote: >> Hi Vyom, >> >> That fixes the include file problem but as this was obviously not tested on >> non-Linux have you checked that the rest of the fix for 8266610 works okay >> on non-Linux? >> >>

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
On Mon, 10 May 2021 07:39:44 GMT, David Holmes wrote: > Hi Vyom, > > That fixes the include file problem but as this was obviously not tested on > non-Linux have you checked that the rest of the fix for 8266610 works okay on > non-Linux? > > Thanks, > David Hi David, Original fix was inten

RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

2021-05-10 Thread Vyom Tewari
this change will include the below headers files to Linux only. #include #include - Commit messages: - fixed the indentation - added the additional check so that modified code get executed on linux only. - JDK-8266797: Fix for 8266610 breaks the build on macos Changes: https:

Re: RFR: 8266610: Method RandomAccessFile#length() returns 0 for block devices on linux. [v2]

2021-05-09 Thread Vyom Tewari
On Fri, 7 May 2021 12:17:11 GMT, Vyom Tewari wrote: >> RandomAccessFile.length() method for block device return always 0 > > Vyom Tewari has updated the pull request incrementally with one additional > commit since the last revision: > > fixed assigning -1 to uint64_t

Re: RFR: 8266610: Method RandomAccessFile#length() returns 0 for block devices on linux. [v2]

2021-05-09 Thread Vyom Tewari
On Sun, 9 May 2021 06:26:13 GMT, Alan Bateman wrote: > Could required os = linux added for > test/jdk/java/nio/channels/FileChannel/BlockDeviceSize.java? As it is > decribed only run as linux. Sure, this is separate issue(https://bugs.openjdk.java.net/browse/JDK-8150539). We will fix it separ

Integrated: 8266610: Method RandomAccessFile#length() returns 0 for block devices on linux.

2021-05-09 Thread Vyom Tewari
On Fri, 7 May 2021 06:16:07 GMT, Vyom Tewari wrote: > RandomAccessFile.length() method for block device return always 0 This pull request has now been integrated. Changeset: 69b96f9a Author: Vyom Tewari URL: https://git.openjdk.java.net/jdk/com

Re: RFR: 8266610: Method RandomAccessFile#length() returns 0 for block devices on linux. [v2]

2021-05-08 Thread Vyom Tewari
On Sun, 9 May 2021 01:39:40 GMT, Hui Shi wrote: > Could required os = linux added for > test/jdk/java/nio/channels/FileChannel/BlockDeviceSize.java? As it is > decribed only run as linux. Sure, this is separate issue(https://bugs.openjdk.java.net/browse/JDK-8150539). We will fix it separately

Re: RFR: 8266610: Method RandomAccessFile#length() returns 0 for block devices on linux. [v2]

2021-05-07 Thread Vyom Tewari
> RandomAccessFile.length() method for block device return always 0 Vyom Tewari has updated the pull request incrementally with one additional commit since the last revision: fixed assigning -1 to uint64_t - Changes: - all: https://git.openjdk.java.net/jdk/pull/3914/fi

RFR: 8266610: Method RandomAccessFile#length() returns 0 for block devices on linux.

2021-05-06 Thread Vyom Tewari
RandomAccessFile.length() method for block device return always 0 - Commit messages: - JDK-8266610: Method RandomAccessFile#length() returns 0 for block devices on linux. Changes: https://git.openjdk.java.net/jdk/pull/3914/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&p

Re: RFR: 8260621: (jrtfs) ThreadLocal memory leak in ImageBufferCache when using jrtfs

2021-05-04 Thread Vyom Tewari
On Tue, 4 May 2021 09:05:38 GMT, Athijegannathan Sundararajan wrote: > Instead of BufferReference class, Map.Entry is used as pair implementation. > This avoids the metaspace leak seen via thread local. looks OK to me. - Marked as reviewed by vtewari (Committer). PR: https://git.

Re: RFR: JDK-8265496: improve null check in DeflaterOutputStream/InflaterInputStream [v2]

2021-04-25 Thread Vyom Tewari
On Mon, 26 Apr 2021 02:36:54 GMT, Hamlin Li wrote: >> code like below will create Deflater before null check, although it's not a >> real mem leak, but it's better to do null check before new Deflater. >> >> try { >> DeflaterOutputStream dos = new DeflaterOutputStream(null);

Re: RFR: 8263108: Class initialization deadlock in java.lang.constant [v4]

2021-03-16 Thread Vyom Tewari
On Tue, 16 Mar 2021 02:37:24 GMT, Jaikiran Pai wrote: >> Can I please get a review for this proposed patch for the issue reported in >> https://bugs.openjdk.java.net/browse/JDK-8263108? >> >> As noted in that issue, the `java.lang.constant.DynamicConstantDesc` and >> `java.lang.constant.Consta

Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass

2021-03-14 Thread Vyom Tewari
On Sat, 20 Feb 2021 12:17:32 GMT, Сергей Цыпанов wrote: > This is a very simple and trivial improvement about getting rid of pointless > char wrapping into array LGTM - Marked as reviewed by vtewari (Committer). PR: https://git.openjdk.java.net/jdk/pull/2660

Re: RFR: 8263108: Class initialization deadlock in java.lang.constant [v2]

2021-03-09 Thread Vyom Tewari
On Wed, 10 Mar 2021 02:15:28 GMT, Jaikiran Pai wrote: >> Can I please get a review for this proposed patch for the issue reported in >> https://bugs.openjdk.java.net/browse/JDK-8263108? >> >> As noted in that issue, the `java.lang.constant.DynamicConstantDesc` and >> `java.lang.constant.Consta

Re: [RFR] 8214440: ldap over a TLS connection negotiate failed with "javax.net.ssl.SSLPeerUnverifiedException: hostname of the server '' does not match the hostname in the server's certificate"

2019-01-08 Thread vyom tewari
Hi Rob, Thanks for fixing this issue, please use hostname.isEmpty(), instead of "".equal(hostname). I have a question to you, why we are converting null to empty string("") in LdapDnsProvider ?. If you see the java doc it tells that domainname can be null /** * Construct an LdapDnsProvi

Re: [12] RFR JDK-8214241: Problem list com/sun/jndi/ldap/LdapTimeoutTest.java on all platforms

2018-11-22 Thread vyom tewari
looks OK to me. Vyom On 23/11/18 9:28 AM, Amy Lu wrote: Please review the patch to problem list com/sun/jndi/ldap/LdapTimeoutTest.java This test fails frequently, originally observed on linux-x64 but recently also on other platforms, and should be problem listed before JDK-8151678 fixed.

Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

2018-10-30 Thread vyom tewari
Hi Rob, Overall your code looks OK to me, In LdapDnsProviderService.java please make 'service' as volatile otherwise 'getInstance()' may not work as expected. I can see you implemented DCL but it may not work if 'service' is not declare volatile. If you like, you can us the initialization 'O

Re: RFR[12] JDK-8211107: LDAPS communication failure with jdk 1.8.0_181

2018-10-01 Thread vyom tewari
Hi Prasad, Change looks good to me, can you please put some comment why we are not performing "ssl handshake" if connectTime <= 0. This will help future maintainer of this code. Thanks, Vyom On Tuesday 02 October 2018 06:39 AM, Prasadrao Koppula wrote: Hi, Could you please review thi

Re: [12] RFR 8210339: Add 10 JNDI tests to com/sun/jndi/dns/FedTests/

2018-09-20 Thread vyom tewari
Hi Chris, tests  looks good to me. Thanks, On Tuesday 04 September 2018 12:00 PM, Chris Yin wrote: Please review the changes to add 10 JNDI tests to com/sun/jndi/dns/FedTests/ in OpenJDK, thanks bug: https://bugs.openjdk.java.net/browse/JDK-8210339 webrev: http://cr.openjdk.java.net/~xyin/

Re: RFR:8205330 InitialDirContext ctor sometimes throws NPE if the server has sent a disconnection

2018-09-11 Thread vyom tewari
Hi Chris,Daniel, Please find the latest patch( http://cr.openjdk.java.net/~vtewari/8205330/webrev0.1/index.html ). I did the additional cleanup(removed redundant null check) as you suggested. Thanks, Vyom On Monday 10 September 2018 09:03 PM, Daniel Fuchs wrote: On 10/09/2018 15:53, vyom

Re: RFR:8205330 InitialDirContext ctor sometimes throws NPE if the server has sent a disconnection

2018-09-10 Thread vyom tewari
-    conn.readTimeout = readTimeout; +    if (conn != null) +    conn.readTimeout = readTimeout; } } -Chris. On 07/09/18 17:47, Daniel Fuchs wrote: Hi Vyom, Please see inline... I've elided some parts... On 07/09/2018 10:11, vyom tewari wrote: On

Re: RFR:8205330 InitialDirContext ctor sometimes throws NPE if the server has sent a disconnection

2018-09-07 Thread vyom tewari
f closing it before taking it out of the pool, etc... Changing order will not help, i think closing the  same connection is not a problem  but setting `LdapClient.conn'  to null asynchronously is causing NPE. Please do let me know if i am missing something. best regards, -- daniel On 04/09/

Re: RFR:8205330 InitialDirContext ctor sometimes throws NPE if the server has sent a disconnection

2018-09-04 Thread vyom tewari
On Friday 24 August 2018 08:52 PM, Chris Hegarty wrote: Hi Vyom, On 24/08/18 11:35, vyom tewari wrote: Hi All, Please review this simple fix below webrev: http://cr.openjdk.java.net/~vtewari/8205330/webrev0.0/index.html bugid: https://bugs.openjdk.java.net/browse/JDK-8205330 This fix

Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

2018-09-04 Thread vyom tewari
Hi Rob, Code change looks good to me. Thanks, Vyom On Tuesday 04 September 2018 03:18 AM, Rob McKenna wrote: Hi folks, I'd like to get a re-review of this change: https://bugs.openjdk.java.net/browse/JDK-8139965 http://cr.openjdk.java.net/~robm/8139965/webrev/ In case the original has gon

Re: [12] RFR 8209773: Refactor shell test javax/naming/module/basic.sh to java

2018-08-30 Thread vyom tewari
Hi Chris, The refactored  java class (RunBasic.java) looks good to me. Thanks, Vyom On Tuesday 28 August 2018 11:20 AM, Chris Yin wrote: Please have a review for below change to refactor shell test javax/naming/module/basic.sh to plain java, no test logic change, old shell script is remove

RFR:8205330 InitialDirContext ctor sometimes throws NPE if the server has sent a disconnection

2018-08-24 Thread vyom tewari
Hi All, Please review this simple fix below webrev: http://cr.openjdk.java.net/~vtewari/8205330/webrev0.0/index.html bugid: https://bugs.openjdk.java.net/browse/JDK-8205330 This fix will resolve the race in LdapClient  where we are explicitly making "null" to LdapClient.conn. Thanks, Vyom

Re: RFR: JDK-8176553 LdapContext follows referrals infinitely ignoring set limit

2018-08-20 Thread Vyom Tewari
On 8/20/2018 4:19 PM, Chris Hegarty wrote: On 19 Aug 2018, at 12:51, vyom tewari wrote: Hi, Please review the below code change. Webrev : http://cr.openjdk.java.net/~vtewari/8176553/webrev0.0/index.html bugid: https://bugs.openjdk.java.net/browse/JDK-8176553 Our all internal tests

Re: [12] RFR 8208542: Add 4 JNDI tests to com/sun/jndi/dns/ListTests/

2018-08-20 Thread vyom tewari
Hi Chris, Latest webrev(.02) looks good to me. One minor comment i will suggest you to  expand "setContext" as you did for other JNDI tests. Thanks, Vyom On Friday 10 August 2018 02:34 PM, Chris Yin wrote: Sorry... another minor revision to handle @Override line and imports place, new web

RFR: JDK-8176553 LdapContext follows referrals infinitely ignoring set limit

2018-08-19 Thread vyom tewari
Hi, Please review the below  code change. Webrev : http://cr.openjdk.java.net/~vtewari/8176553/webrev0.0/index.html bugid    : https://bugs.openjdk.java.net/browse/JDK-8176553 Our  all internal tests are clean, this patch is contributed by Jan Kalina(Redhat). Thanks, Vyom

Re: [12] RFR 8208483: Add 5 JNDI tests to com/sun/jndi/dns/FactoryTests/

2018-08-08 Thread vyom tewari
class as ‘verifyLookupObjectAndValue’, hope that looks better, update new webrev as below, thanks http://cr.openjdk.java.net/~xyin/8208483/webrev.03/ Regards, Chris On 7 Aug 2018, at 4:35 PM, vyom tewari wrote: Hi Chris, Overall latest code looks good to me, one minor comment did you consider

Re: [12] RFR 8208483: Add 5 JNDI tests to com/sun/jndi/dns/FactoryTests/

2018-08-07 Thread vyom tewari
uot;. .Thanks, Vyom On Monday 06 August 2018 03:02 PM, Chris Yin wrote: Hi, Vyom Many thanks for your review and comments, inline and updated webrev as below, thanks http://cr.openjdk.java.net/~xyin/8208483/webrev.01/ On 6 Aug 2018, at 4:12 PM, vyom tewari wrote: Hi Chris, 1-> DirA

Re: [12] RFR 8208483: Add 5 JNDI tests to com/sun/jndi/dns/FactoryTests/

2018-08-06 Thread vyom tewari
On Monday 06 August 2018 03:02 PM, Chris Yin wrote: Hi, Vyom Many thanks for your review and comments, inline and updated webrev as below, thanks http://cr.openjdk.java.net/~xyin/8208483/webrev.01/ On 6 Aug 2018, at 4:12 PM, vyom tewari wrote: Hi Chris, 1-> DirAFactory.j

Re: [12] RFR 8208483: Add 5 JNDI tests to com/sun/jndi/dns/FactoryTests/

2018-08-06 Thread vyom tewari
Hi Chris, 1-> DirAFactory.java, "getIbjectInstance" returns "null"  if it fails to construct object. I will suggest you to throw  some "RuntimeException" instead returning null. If you return null then caller of "getObjectInstance" had to check for null and it will end in lots of boilerplate

Re: [12] RFR 8208279: Add 8 JNDI tests to com/sun/jndi/dns/EnvTests/

2018-08-05 Thread vyom tewari
Hi Chris, Latest webrev looks good to me. Thanks, Vyom On Friday 03 August 2018 02:46 PM, Chris Yin wrote: Hi, Vyom Thank a lot for your review and comments, inline and update new webrev as below http://cr.openjdk.java.net/~xyin/8208279/webrev.02/ On 3 Aug 2018, at 3:45 PM, vyom tewari

Re: [12] RFR 8208279: Add 8 JNDI tests to com/sun/jndi/dns/EnvTests/

2018-08-03 Thread vyom tewari
Hi Chris, Overall looks good to me, couple of minor comment. 1-> ProviderUrlGen.java, Line:100 Exception e can be replaced with specific  exceptions. 2-> In couple of  places we are calling "removeFromEnvironment" on context and spec says it will return "null" if the env is not set. I  see

Re: [12] RFR 8200151: Add 8 JNDI tests to com/sun/jndi/dns/ConfigTests/

2018-07-30 Thread vyom tewari
net/%7Exyin/8200151/webrev.01/> On 26 Jul 2018, at 5:27 PM, vyom tewari <mailto:vyom.tew...@oracle.com>> wrote: Hi Chris, Thanks for writing the tests overall code looks good to me, below are few minor comments. 1`-> Fix tag order, my Netbeans always complains :) . Fixed, thank

Re: [12] RFR 8200151: Add 8 JNDI tests to com/sun/jndi/dns/ConfigTests/

2018-07-26 Thread vyom tewari
Hi Chris, Thanks for writing the tests overall code looks good to me, below are few minor comments. 1`-> Fix tag order, my Netbeans always complains :) . 2->  you make AuthRecursiveBase class as public but i can not see it is being used outside,  i will suggest you to give the default access

Re: [11] RFR: 8206445: JImageListTest.java failed in Windows

2018-07-23 Thread vyom tewari
Hi Srinivas, If runTests() throws exception then  "System.gc()" will not  be invoked. Thanks, Vyom On Monday 23 July 2018 07:46 PM, Srinivas Dama wrote: Hi Alan, As discussed in private conversations, I am pushing below fix for the issue. webrev: http://cr.openjdk.java.net/~sdama/8206445/we

Re: RFR 8207750 : Native handle leak in java.io.WinNTFileSystem.list()

2018-07-17 Thread vyom tewari
Hi Ivan, looks good to me, although i am not the official reviewer. Thanks, Vyom On Wednesday 18 July 2018 05:31 AM, Ivan Gerasimov wrote: Hello! In the file WinNTFileSystem_md.c there is a potential leak of native handle obtained from FindFirstFileW() in the unfortunate event of memory a

Re: RFR 8198882: Add 10 JNDI tests to com/sun/jndi/dns/AttributeTests/

2018-07-13 Thread vyom tewari
/8198882/webrev.02/ <http://cr.openjdk.java.net/%7Exyin/8198882/webrev.02/> On 13 Jul 2018, at 1:46 PM, vyom tewari <mailto:vyom.tew...@oracle.com>> wrote: Hi Chris, Thanks for doing this overall looks good to me, few minor comments. 1->  DNSTestUtils.java, please start the ser

Re: RFR 8198882: Add 10 JNDI tests to com/sun/jndi/dns/AttributeTests/

2018-07-12 Thread vyom tewari
Hi Chris, Thanks for doing this overall looks good to me, few minor comments. 1->  DNSTestUtils.java, please start the server first and then set the "TEST_DNS_SERVER_THREAD". This will not make much difference but we will avoid setting "TEST_DNS_SERVER_THREAD" env variable if server fails to

Re: RFR 8207060 : Memory leak when malloc fails within WITH_UNICODE_STRING block

2018-07-11 Thread vyom tewari
Hi Ivan, Changes looks good to me, nice cleanup. Thanks, Vyom On Wednesday 11 July 2018 09:45 PM, Ivan Gerasimov wrote: Hello! File src/java.base/windows/native/libjava/io_util_md.c In the function pathToNTPath(), memory is allocated with malloc() within a block of macros WITH_UNICODE_STR

Re: [JDK 11] RFR 8187069: The case auto failed with the "java.lang.ClassNotFoundException: IPv6NameserverPlatformParsingTest" exception

2018-06-28 Thread vyom tewari
va.desktop   *          jdk.naming.dns/com.sun.jndi.dns + * @requires os.family != "windows" + * @build IPv6NameserverPlatformParsingTest   * @run main/manual Test6991580   */ Regards, Chris On 28 Jun 2018, at 7:02 PM, vyom tewari <mailto:vyom.tew...@oracle.com>> wrote: Hi Chris, change

Re: [JDK 11] RFR 8187069: The case auto failed with the "java.lang.ClassNotFoundException: IPv6NameserverPlatformParsingTest" exception

2018-06-28 Thread vyom tewari
Hi Chris, change looks good to me. My NetBeans always complains about tag order if it is not correct, as you  adding the new tag i will suggest you to please fix the tag order as well. /*  * @test  * @bug 6991580 8080108 8133035  * @summary IPv6 Nameservers in resolv.conf throws NumberFormatE

Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes

2018-05-24 Thread vyom tewari
On Friday 25 May 2018 11:19 AM, Ivan Gerasimov wrote: Hi Wiijun! On 5/24/18 10:13 PM, Weijun Wang wrote: On May 25, 2018, at 11:58 AM, Ivan Gerasimov wrote: I also wonder whether a smart compiler might not flag code where the errors do infact have the same value: if (errno == 11 || e

Re: [11] RFR: JDK-8202582 : DateTimeFormatterBuilder.parseOffsetBased unnecessarily calls toString()

2018-05-04 Thread vyom tewari
looks good to me. Vyom On Friday 04 May 2018 02:29 PM, Rachna Goel wrote: Hi, Kindly review this small patch for JDK-8202582. https://bugs.openjdk.java.net/browse/JDK-8202582 http://cr.openjdk.java.net/~rgoel/JDK-8202582/webrev/ Fix is to call text.subSequence() before calling toString() o

Re: RFR: [PATCH] 8176553 Fix LDAP referral loop

2018-04-04 Thread Vyom Tewari
On 4/4/2018 8:59 PM, Jan Kalina wrote: Note: Test is not included, as it would require running LDAP server. (existing ldap tests in JDK use only mocks of the server) The patch was manually verified using reproducer attached to issue. Hi Jan, I ran the reproducer long back on my Linux box(Ubu

Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

2017-12-07 Thread vyom tewari
Thanks, Vyom On Wednesday 06 December 2017 11:54 PM, Rob McKenna wrote: Thanks Vyom, these should be fixed in: http://cr.openjdk.java.net/~robm/8160768/webrev.07/ Comments inline: On 06/12/17 22:14, vyom tewari wrote: Hi Rob, Please find below comments. DefaultLdapDnsProvider.java  line 35

Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

2017-12-06 Thread vyom tewari
Hi Rob, Please find below comments. DefaultLdapDnsProvider.java  line 35:     convert it to for-each code will be more readable. LdapDnsProviderService.java  line 21: constant name does not follow Java naming convention, there are other places as well please fix it. getInstance() is alread

Re: Fix inconsistent behavior of java.nio.file.attribute.BasicFileAttributes.lastModifiedTime()

2017-11-24 Thread vyom tewari
Hi Martin, which specific Operating system are you observing this issue ?. Mostly all modern OS's supports higher precision time stamp. Thanks, Vyom On Friday 24 November 2017 08:23 PM, Schaef, Martin wrote: We are experiencing a problem with the following test case: https://github.com/ecl

Re: RFR 8145635 : Add TCP_QUICKACK socket option

2017-10-12 Thread vyom tewari
line 34:  using JNU_ThrowByNameWithLastError directly is sufficient; if the errno does not have a string unix supplies "unknown error nnn". Regards, Roger On 10/10/2017 2:58 PM, Chris Hegarty wrote: Vyom, What about suggestion 1) below, the name of the socket option? -Chris. On 2

Re: RFR 8145635 : Add TCP_QUICKACK socket option

2017-09-27 Thread vyom tewari
Hegarty wrote: Vyom, On 11 Sep 2017, at 16:38, vyom tewari wrote: Hi All, As jdk.net API already moved out of java.base, Please review the below code change for jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8145635 Webrev: http://cr.openjdk.java.net/~vtewari/8145635/webrev0.1/index.html

Re: RFR 8145635 : Add TCP_QUICKACK socket option

2017-09-22 Thread vyom tewari
ping!! Vyom On Monday 11 September 2017 09:08 PM, vyom tewari wrote: Hi All, As jdk.net API already moved out of java.base, Please review the below code change for jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8145635 Webrev: http://cr.openjdk.java.net/~vtewari/8145635/webrev0.1

Re: RFR[10]:8159526 Deprivilege jdk.httpserver

2017-09-13 Thread vyom tewari
On Tuesday 12 September 2017 09:16 PM, Sean Mullan wrote: On 9/12/17 4:06 AM, vyom tewari wrote: Hi, Please review the below code change. BugId: https://bugs.openjdk.java.net/browse/JDK-8159526 Webrev-1: http://cr.openjdk.java.net/~vtewari/8159526/jdk/webrev/index.html Can you put the

Re: RFR[10]:8159526 Deprivilege jdk.httpserver

2017-09-12 Thread vyom tewari
On Tuesday 12 September 2017 02:12 PM, Alan Bateman wrote: On 12/09/2017 09:06, vyom tewari wrote: Hi, Please review the below code change. BugId: https://bugs.openjdk.java.net/browse/JDK-8159526 Webrev-1: http://cr.openjdk.java.net/~vtewari/8159526/jdk/webrev/index.html Webrev-2: http

RFR[10]:8159526 Deprivilege jdk.httpserver

2017-09-12 Thread vyom tewari
Hi, Please review the below code change. BugId: https://bugs.openjdk.java.net/browse/JDK-8159526 Webrev-1: http://cr.openjdk.java.net/~vtewari/8159526/jdk/webrev/index.html Webrev-2: http://cr.openjdk.java.net/~vtewari/8159526/root/webrev/index.html Code change will De-privilege jdk.httpserve

Re: RFR 8145635 : Add TCP_QUICKACK socket option

2017-09-11 Thread vyom tewari
Hi All, As jdk.net API already moved out of java.base, Please review the below code change for jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8145635 Webrev: http://cr.openjdk.java.net/~vtewari/8145635/webrev0.1/index.html Thanks, Vyom On Wednesday 24 February 2016 03:16 PM, Alan Ba

Re: [10] 8177806: ldapContext.unbind not handling ldap entry with esc char

2017-07-06 Thread Vyom Tewari
Hi Rob, Code change looks OK, below are few comments on test case. 1-> Remove System.exit(1) and throw Exception from main if test fails. 2-> Put srv.close() in finally block, in case of exception close will not be called. 3-> Do you need Thread.sleep(500) ?. I think it is not required. 4->

Re: RFR(10) 8176192: Incorrect usage of Iterator in Java 8 In com.sun.jndi.ldap.EventSupport.removeNamingListener

2017-06-13 Thread Vyom Tewari
, thereby simplifying the code. RemoveNamingListenerTest — 50 private static Exception exception; This should be marked volatile. Paul. On 13 Jun 2017, at 01:30, Vyom Tewari wrote: Hi All, Please find the code change for below issue. BugId : https://bugs.openjdk.java.net/browse

RFR(10) 8176192: Incorrect usage of Iterator in Java 8 In com.sun.jndi.ldap.EventSupport.removeNamingListener

2017-06-13 Thread Vyom Tewari
Hi All, Please find the code change for below issue. BugId : https://bugs.openjdk.java.net/browse/JDK-8176192 Webrev : http://cr.openjdk.java.net/~vtewari/8176192/webrev0.0/index.html If the hashtable is structurally modified while enumerating over the values then the results of enumer

Re: JDK10: RFR(xxs): 8181207: 8177809 breaks AIX 5.3, 6.1 builds

2017-05-31 Thread Vyom Tewari
Hi Thomas, one minor comment apart from what Christoph gave, you can combine the the two explicit assignment statements to one in all three places. rv = (jlong)sb.st_mtimespec.tv_sec * 1000; rv += (jlong)sb.st_mtimespec.tv_nsec / 100; to rv= (jlong)sb.st_mtimespec.tv_sec * 1000 +(jlong

Re: JDK10: RFR(xxs): 8181207: 8177809 breaks AIX 5.3, 6.1 builds

2017-05-30 Thread Vyom Tewari
Hi Thomas, Change looks good to me, but i am not official reviewer. Thanks, Vyom On Tuesday 30 May 2017 03:16 PM, Thomas Stüfe wrote: Hi all, may I have please a review for this tiny change: Bug: https://bugs.openjdk.java.net/browse/JDK-8181207 webrev: http://cr.openjdk.java.net/~stuefe/we

Re: JDK 10 RFR of 8177809: File.lastModified() is losing milliseconds (always ends in 000)

2017-05-18 Thread Vyom Tewari
Hi Brian, looks good to me, although i am not JDK10 reviewer, one minor comment i think you can combine the below two statements to one rv = (jlong)sb.st_mtimespec.tv_sec * 1000; rv += (jlong)sb.st_mtimespec.tv_nsec / 100; rv= (jlong)sb.st_mtimespec.tv_sec * 1000 +(jlong)sb.st_mtimespec.

Re: (9) (docs only) RFR: 8180176: Broken javadoc links in java.logging and java.naming

2017-05-12 Thread Vyom Tewari
Hi Daniel, Changes looks good, i was under impression that if {@extLink } description is multi word then you have to put it under double quotes like {xtLink jndi_overview "JNDI documentation"} but looks like that is not the case ExtLink.java will take care of this. Thanks, Vyom On Friday

Re: RFR: 8178298 (LdapLoginModule)Migrate the JNDI properties technote to javadoc doc-files

2017-04-27 Thread Vyom Tewari
On Tuesday 25 April 2017 08:41 PM, Chris Hegarty wrote: Vyom, On 24 Apr 2017, at 10:15, Vyom Tewari wrote: Hi All, Please review the simple doc fix. Webrev : http://cr.openjdk.java.net/~vtewari/8178298/webrev0.0/index.html Bugid : https://bugs.openjdk.java.net/browse/JDK-8178298

Re: RFR: 8178298 (LdapLoginModule)Migrate the JNDI properties technote to javadoc doc-files

2017-04-25 Thread Vyom Tewari
On Tuesday 25 April 2017 08:41 PM, Chris Hegarty wrote: Vyom, On 24 Apr 2017, at 10:15, Vyom Tewari wrote: Hi All, Please review the simple doc fix. Webrev : http://cr.openjdk.java.net/~vtewari/8178298/webrev0.0/index.html Bugid : https://bugs.openjdk.java.net/browse/JDK-8178298

RFR: 8178298 (LdapLoginModule)Migrate the JNDI properties technote to javadoc doc-files

2017-04-24 Thread Vyom Tewari
Hi All, Please review the simple doc fix. Webrev : http://cr.openjdk.java.net/~vtewari/8178298/webrev0.0/index.html Bugid : https://bugs.openjdk.java.net/browse/JDK-8178298 Note, this patch depends on "JDK-8178725". Thanks, Vyom

Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

2017-02-03 Thread Vyom Tewari
Hi Rob, Most of the code changes looks fine except new code will throw NPE if "ServiceLocator.mapDnToDomainName(ldapUrl.getDN())" is not able to map. you have to check for null in LdapCtxFactory.java class as follows. if(ServiceLocator.mapDnToDomainName(ldapUrl.getDN())!=null){

Re: Review request - 8169465: Deadlock in com.sun.jndi.ldap.pool.Connections

2016-12-15 Thread Vyom Tewari
Hi Rob, Code changes looks good to me. Thanks, Vyom On Thursday 15 December 2016 08:27 PM, Rob McKenna wrote: Gah, thanks Daniel. I originally worked this fix on 8u and neglected to remove synchronized when forward porting. I've been having a few discussions on altering how we do locking in

Re: RFR 4823133: RandomAccessFile.length() is not thread-safe

2015-12-10 Thread Vyom Tewari
+nio-dev On 12/10/2015 5:54 PM, David Holmes wrote: Hi Vyom, Should this be reviewed by the NIO folk? It isn't obvious whether this change impacts the interaction with the associated FileChannel - see also https://bugs.openjdk.java.net/browse/JDK-6371642 Thanks, David On 10/12/2015 8:52 PM

RFR 8068887 : java.lang.Throwable could use Collections.emptyList for suppressedException

2015-10-12 Thread Vyom Tewari
Hi All, Please review my changes for below bug. Bug:JDK-8068887 : java.lang.Throwable could use Collections.emptyList for suppressedException Webrev: http://cr.openjdk.java.net/~vtewari/8068887/webrev.00/webrev/ This change ensure that fewer classes are loaded in a simple(hello

RFR 8038075 : JNI warnings in jdk/src/share/native/sun/misc/VMSupport.c

2015-09-28 Thread Vyom Tewari
Hi All, Please review my changes for below bug. Bug: JDK-8038075 : JNI warnings in jdk/src/share/native/sun/misc/VMSupport.c Webrev: http://cr.openjdk.java.net/~dfuchs/vyom/8038075/webrev.00/ This change ensure that t

RFR 8073542 : File Leak in jdk/src/java/base/unix/native/libnet/PlainDatagramSocketImpl.c

2015-09-16 Thread Vyom Tewari
Hi All, Please review my changes for below bug. Bug: JDK-8073542 : File Leak in jdk/src/java/base/unix/native/libnet/PlainDatagramSocketImpl.c Webrev: http://cr.openjdk.java.net/~dfuchs/vyom/8073542/webrev.00 This change ensure that if "setsocketopt" fails we close the corresponding fd

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-09 Thread Vyom Tewari
. regards Mark On 07/09/2015 14:29, Ivan Gerasimov wrote: Thanks! It looks good to me now. Sincerely yours, Ivan On 07.09.2015 16:08, Vyom Tewari wrote: Hi All, Please find the latest diff, which contains the latest fix. http://cr.openjdk.java.net/~dfuchs/vyom/8080402/webrev.02/ Thanks,

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Vyom Tewari
Hi All, Please find the latest diff, which contains the latest fix. http://cr.openjdk.java.net/~dfuchs/vyom/8080402/webrev.02/ Thanks, Vyom On 9/7/2015 3:48 PM, Alan Bateman wrote: On 07/09/2015 10:26, Vyom Tewari wrote: Hi everyone, Can you please review my changes for below bug. Bug

RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Vyom Tewari
Hi everyone, Can you please review my changes for below bug. Bug: JDK-8080402 : File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java Webrev: http://cr.openjdk.java.net/~dfuchs/vyom/8080402/webrev.01/ This change ensure that if close() fails we throw correct io exceptio

RFR 8080486: JNI exception pending in jdk/src/java.base/windows/native/libnet/DualStackPlainSocketImpl.c

2015-09-02 Thread Vyom Tewari
Hi everyone, Can you please review my changes for below bug. Bug: JDK-8080486 : JNI exception pending in jdk/src/java.base/windows/native/libnet/DualStackPlainSocketImpl.c Webrev: http://cr.openjdk.java.net/~dfuchs/vyom/8080486/webrev.01 This change ensure that we immediately return if

RFR <8064470 > JNI exception pending in jdk/src/java/base/unix/native/libjava/FileDescriptor_md.c

2015-09-01 Thread Vyom Tewari
Hi everyone, Can you please review my changes for below bug. Bug: JDK-8064470 : JNI exception pending in jdk/src/java/base/unix/native/libjava/FileDescriptor_md.c Webrev: http://cr.openjdk.java.net/~dfuchs/vyom/8064470/webrev.01/ This change ensure that we immediately return if an except