Suspicious 'completed' variable in ResponseContent.UnknownLengthBodyParser#accept
Hello
During investigation of results of IDEA inspections I found suspicious
code in a method
'jdk.internal.net.http.ResponseContent.UnknownLengthBodyParser#accept'
https://github.com/openjdk/jdk/blob/master/src/java.net.http/share/classes/jdk/internal/net/http/ResponseContent.java#L492
boolean completed = false;
try {
if (debug.on())
debug.log("Parser got %d bytes ", b.remaining());
if (b.hasRemaining()) {
// only reduce demand if we actually push something.
// we would not have come here if there was no
// demand.
boolean hasDemand = sub.demand().tryDecrement();
assert hasDemand;
breceived += b.remaining();
pusher.onNext(List.of(b.asReadOnlyBuffer()));
}
} catch (Throwable t) {
if (debug.on()) debug.log("Unexpected exception", t);
closedExceptionally = t;
if (!completed) {
onComplete.accept(t);
}
}
Variable 'completed' has an initial value 'false' and never assigned again.
Then it's checked in the 'catch' section.
It seems it should be set to 'true' somewhere.
Andrey Turbanov
Re: Suspicious 'completed' variable in ResponseContent.UnknownLengthBodyParser#accept
Hi Andrey,
Thanks for the notice. I believe the variable is not needed,
it's probably a copy-paste error. You will see that such a
variable is also declared in the FixedLengthBodyParser where
it's actually used.
The UnknownLenghtParser will parse the bytes until the
connection is closed (or an exception occurs) - so
`completed` is not really needed there.
best regards,
-- daniel
On 18/08/2021 10:38, Andrey Turbanov wrote:
Hello
During investigation of results of IDEA inspections I found suspicious
code in a method
'jdk.internal.net.http.ResponseContent.UnknownLengthBodyParser#accept'
https://github.com/openjdk/jdk/blob/master/src/java.net.http/share/classes/jdk/internal/net/http/ResponseContent.java#L492
boolean completed = false;
try {
if (debug.on())
debug.log("Parser got %d bytes ", b.remaining());
if (b.hasRemaining()) {
// only reduce demand if we actually push something.
// we would not have come here if there was no
// demand.
boolean hasDemand = sub.demand().tryDecrement();
assert hasDemand;
breceived += b.remaining();
pusher.onNext(List.of(b.asReadOnlyBuffer()));
}
} catch (Throwable t) {
if (debug.on()) debug.log("Unexpected exception", t);
closedExceptionally = t;
if (!completed) {
onComplete.accept(t);
}
}
Variable 'completed' has an initial value 'false' and never assigned again.
Then it's checked in the 'catch' section.
It seems it should be set to 'true' somewhere.
Andrey Turbanov
RFR: 8272626: Avoid C-style array declarations in java.*
C-style array declarations generate noisy warnings in IDEs, et.c. This patch cleans up all java.* packages. (Copyrights intentionally not updated due the triviality of most changes) - Commit messages: - Avoid C-style array declarations in java packages Changes: https://git.openjdk.java.net/jdk/pull/5161/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5161&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272626 Stats: 140 lines in 54 files changed: 0 ins; 0 del; 140 mod Patch: https://git.openjdk.java.net/jdk/pull/5161.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5161/head:pull/5161 PR: https://git.openjdk.java.net/jdk/pull/5161
Re: RFR: 8272626: Avoid C-style array declarations in java.*
On Wed, 18 Aug 2021 10:07:35 GMT, Claes Redestad wrote: > C-style array declarations generate noisy warnings in IDEs, et.c. This patch > cleans up all java.* packages. > > (Copyrights intentionally not updated due the triviality of most changes) Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5161
Re: RFR: 8272626: Avoid C-style array declarations in java.*
On Wed, 18 Aug 2021 10:07:35 GMT, Claes Redestad wrote: > C-style array declarations generate noisy warnings in IDEs, et.c. This patch > cleans up all java.* packages. > > (Copyrights intentionally not updated due the triviality of most changes) Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5161
Re: RFR: 8272626: Avoid C-style array declarations in java.*
On Wed, 18 Aug 2021 10:07:35 GMT, Claes Redestad wrote: > C-style array declarations generate noisy warnings in IDEs, et.c. This patch > cleans up all java.* packages. > > (Copyrights intentionally not updated due the triviality of most changes) Thanks for reviewing, Daniel and Alan! - PR: https://git.openjdk.java.net/jdk/pull/5161
Integrated: 8272626: Avoid C-style array declarations in java.*
On Wed, 18 Aug 2021 10:07:35 GMT, Claes Redestad wrote: > C-style array declarations generate noisy warnings in IDEs, et.c. This patch > cleans up all java.* packages. > > (Copyrights intentionally not updated due the triviality of most changes) This pull request has now been integrated. Changeset: 30b0f820 Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/30b0f820cec12b6da62229fe78a528ab3ad0d134 Stats: 140 lines in 54 files changed: 0 ins; 0 del; 140 mod 8272626: Avoid C-style array declarations in java.* Reviewed-by: dfuchs, alanb - PR: https://git.openjdk.java.net/jdk/pull/5161
Re: RFR: 8272626: Avoid C-style array declarations in java.*
On Wed, 18 Aug 2021 10:07:35 GMT, Claes Redestad wrote: > C-style array declarations generate noisy warnings in IDEs, et.c. This patch > cleans up all java.* packages. > > (Copyrights intentionally not updated due the triviality of most changes) 34 Minutes from proposed to integrated! Its hard to argue with the efficiency, but only 1 timezone of developers had a chance to review or even be aware of the change. - PR: https://git.openjdk.java.net/jdk/pull/5161
Re: RFR: 8272626: Avoid C-style array declarations in java.*
On 8/18/2021 6:20 AM, Roger Riggs wrote: On Wed, 18 Aug 2021 10:07:35 GMT, Claes Redestad wrote: C-style array declarations generate noisy warnings in IDEs, et.c. This patch cleans up all java.* packages. (Copyrights intentionally not updated due the triviality of most changes) 34 Minutes from proposed to integrated! Its hard to argue with the efficiency, but only 1 timezone of developers had a chance to review or even be aware of the change. I don't think removing use of this archaic language feature, which doesn't change semantics, should be in any way controversial and is long overdue. -Joe
