On Monday 14 April 2008 07:21, nextgens at freenetproject.org wrote:
> Author: nextgens
> Date: 2008-04-14 06:21:05 +0000 (Mon, 14 Apr 2008)
> New Revision: 19304
>
> Modified:
> trunk/freenet/src/freenet/client/async/ClientRequestScheduler.java
> trunk/freenet/src/freenet/keys/ClientCHKBlock.java
> trunk/freenet/src/freenet/keys/FreenetURI.java
> trunk/freenet/src/freenet/node/FailureTable.java
> trunk/freenet/src/freenet/node/PeerManager.java
> trunk/freenet/src/freenet/node/PeerNode.java
> trunk/freenet/src/freenet/support/LoggerHookChain.java
> trunk/freenet/src/freenet/support/SectoredRandomGrabArray.java
> Log:
> some paranoia code to prevent race conditions and out-of-bound exceptions
>
> Modified: trunk/freenet/src/freenet/client/async/ClientRequestScheduler.java
> ===================================================================
> --- trunk/freenet/src/freenet/client/async/ClientRequestScheduler.java
2008-04-14 05:28:57 UTC (rev 19303)
> +++ trunk/freenet/src/freenet/client/async/ClientRequestScheduler.java
2008-04-14 06:21:05 UTC (rev 19304)
> @@ -531,10 +531,11 @@
> }
> } else {
> SendableGet[] gets = (SendableGet[]) o;
> - SendableGet[] newGets = new
> SendableGet[gets.length-1];
> + final int getsLength = gets.length;
> + SendableGet[] newGets = new
> SendableGet[getsLength > 1 ? getsLength-1 :
0];
If gets.length == 0 then handle that properly please. But IMHO it shouldn't
happen.
> boolean found = false;
> int x = 0;
> - for(int j=0;j<gets.length;j++) {
> + for(int j=0;j<getsLength;j++) {
Congratulations, your pointless paranoia code has just introduced a real live
ArrayIndexOutOfBoundsException! For extra points, tell me where!
> if(gets[j] == getter) {
> found = true;
> continue;
> @@ -542,7 +543,7 @@
> if(j == newGets.length) {
> if(!found) {
> if(complain)
> -
> Logger.normal(this, "Not found: "+getter+" for "+key+" removing
("+gets.length+" getters)");
> +
> Logger.normal(this, "Not found: "+getter+" for "+key+" removing
("+getsLength+" getters)");
> return; // not here
> }
> }
> @@ -555,7 +556,7 @@
> } else if(x == 1) {
> pendingKeys.put(key, newGets[0]);
> } else {
> - if(x != gets.length-1) {
> + if(x != -1) {
x can't be -1.
> SendableGet[] newNewGets = new
> SendableGet[x];
> System.arraycopy(newGets, 0,
> newNewGets, 0, x);
> newGets = newNewGets;
>
> Modified: trunk/freenet/src/freenet/keys/ClientCHKBlock.java
> ===================================================================
> --- trunk/freenet/src/freenet/keys/ClientCHKBlock.java 2008-04-14
> 05:28:57
UTC (rev 19303)
> +++ trunk/freenet/src/freenet/keys/ClientCHKBlock.java 2008-04-14
> 06:21:05
UTC (rev 19304)
> @@ -89,7 +89,8 @@
> throw new CHKDecodeException("Crypto key too short");
> cipher.initialize(key.cryptoKey);
> PCFBMode pcfb = PCFBMode.create(cipher);
> - byte[] hbuf = new byte[headers.length > 2 ? headers.length-2 : 0];
> + int headersLength = headers.length;
> + byte[] hbuf = new byte[headersLength > 2 ? headersLength-2 : 0];
This is pointless.
> System.arraycopy(headers, 2, hbuf, 0, hbuf.length);
> byte[] dbuf = new byte[data.length];
> System.arraycopy(data, 0, dbuf, 0, data.length);
>
> Modified: trunk/freenet/src/freenet/keys/FreenetURI.java
> ===================================================================
> --- trunk/freenet/src/freenet/keys/FreenetURI.java 2008-04-14 05:28:57 UTC
(rev 19303)
> +++ trunk/freenet/src/freenet/keys/FreenetURI.java 2008-04-14 06:21:05 UTC
(rev 19304)
> @@ -434,8 +434,9 @@
> */
> public FreenetURI popMetaString() {
> String[] newMetaStr = null;
> - if ((metaStr != null) && (metaStr.length > 1)) {
> - newMetaStr = new String[metaStr.length - 1];
> + final int metaStrLength = metaStr.length;
You've just created an NPE, congrats!
> + if ((metaStr != null) && (metaStrLength > 1)) {
> + newMetaStr = new String[metaStrLength - 1];
> System.arraycopy(metaStr, 1, newMetaStr, 0,
> newMetaStr.length);
> }
> return setMetaString(newMetaStr);
>
> Modified: trunk/freenet/src/freenet/node/FailureTable.java
> ===================================================================
> --- trunk/freenet/src/freenet/node/FailureTable.java 2008-04-14 05:28:57 UTC
(rev 19303)
> +++ trunk/freenet/src/freenet/node/FailureTable.java 2008-04-14 06:21:05 UTC
(rev 19304)
> @@ -154,12 +154,13 @@
> if(logMINOR) Logger.minor(this, "Deleting "+offer+"
> from "+this);
> synchronized(this) {
> int idx = -1;
> - for(int i=0;i<offers.length;i++) {
> + final int offerLength = offers.length;
> + for(int i=0;i<offerLength;i++) {
Also pointless, but at least it doesn't break anything...
> if(offers[i] == offer) idx = i;
> }
> if(idx == -1) return;
> if(offers.length > 1) {
> - BlockOffer[] newOffers = new
> BlockOffer[offers.length-1];
> + BlockOffer[] newOffers = new
> BlockOffer[offerLength > 1 ?
offerLength-1 : 0];
Ditto.
> if(idx > 0)
> System.arraycopy(offers, 0,
> newOffers, 0, idx);
> if(idx < newOffers.length)
>
> Modified: trunk/freenet/src/freenet/node/PeerManager.java
> ===================================================================
> --- trunk/freenet/src/freenet/node/PeerManager.java 2008-04-14 05:28:57 UTC
(rev 19303)
> +++ trunk/freenet/src/freenet/node/PeerManager.java 2008-04-14 06:21:05 UTC
(rev 19304)
> @@ -319,7 +319,8 @@
> connectedPeers = newConnectedPeers;
>
> // removing from myPeers
> - PeerNode[] newMyPeers = new PeerNode[myPeers.length-1];
> + int myPeerLength = myPeers.length;
> + PeerNode[] newMyPeers = new PeerNode[myPeerLength > 1 ?
myPeerLength-1 : 0];
We've already checked whether it's in myPeers. If we've reached this point, it
is, so we don't need to check here.
> int positionInNewArray = 0;
> for(int i=0;i<myPeers.length;i++) {
> if(myPeers[i]!=pn){
>
> Modified: trunk/freenet/src/freenet/node/PeerNode.java
> ===================================================================
> --- trunk/freenet/src/freenet/node/PeerNode.java 2008-04-14 05:28:57 UTC
(rev 19303)
> +++ trunk/freenet/src/freenet/node/PeerNode.java 2008-04-14 06:21:05 UTC
(rev 19304)
> @@ -3637,8 +3637,9 @@
> }
>
> public synchronized boolean completedAnnounce(long uid) {
> - if(runningAnnounceUIDs.length == 0) return false;
> - long[] newList = new long[runningAnnounceUIDs.length - 1];
> + final int runningAnnounceUIDsLength =
> runningAnnounceUIDs.length;
> + if(runningAnnounceUIDsLength < 1) return false;
> + long[] newList = new long[runningAnnounceUIDsLength - 1];
Ummm, this is getting really silly... Please don't code on Freenet when unable
to walk without falling over...
> int x = 0;
> for(int i=0;i<runningAnnounceUIDs.length;i++) {
> if(i == runningAnnounceUIDs.length) return false;
>
> Modified: trunk/freenet/src/freenet/support/LoggerHookChain.java
> ===================================================================
> --- trunk/freenet/src/freenet/support/LoggerHookChain.java 2008-04-14
05:28:57 UTC (rev 19303)
> +++ trunk/freenet/src/freenet/support/LoggerHookChain.java 2008-04-14
06:21:05 UTC (rev 19304)
> @@ -59,10 +59,11 @@
> * Remove a hook from the logger.
> */
> public synchronized void removeHook(LoggerHook lh) {
> - LoggerHook[] newHooks = new LoggerHook[hooks.length-1];
> + final int hooksLength = hooks.length;
> + LoggerHook[] newHooks = new LoggerHook[hooksLength > 1 ?
hooksLength-1 : 0];
The length check may actually make sense here. The moving it into a variable
doesn't.
> int x=0;
> boolean removed = false;
> - for(int i=0;i<hooks.length;i++) {
> + for(int i=0;i<hooksLength;i++) {
> if(hooks[i] == lh) {
> removed = true;
> } else {
>
> Modified: trunk/freenet/src/freenet/support/SectoredRandomGrabArray.java
> ===================================================================
> --- trunk/freenet/src/freenet/support/SectoredRandomGrabArray.java
2008-04-14 05:28:57 UTC (rev 19303)
> +++ trunk/freenet/src/freenet/support/SectoredRandomGrabArray.java
2008-04-14 06:21:05 UTC (rev 19304)
> @@ -124,16 +124,17 @@
> // Just because the item is cancelled does not
> necessarily mean the
whole client is.
> // E.g. a segment may return cancelled because it is
> decoding, that
doesn't mean
> // other segments are cancelled. So just go around the
> loop in that
case.
> + final int grabArraysLength = grabArrays.length;
> if(rga.isEmpty()) {
> if(logMINOR)
> Logger.minor(this, "Removing grab array
> "+x+" : "+rga+"
for "+rga.getObject()+" (is empty)");
> Object client = rga.getObject();
> grabArraysByClient.remove(client);
> - RemoveRandomWithObject[] newArray = new
RemoveRandomWithObject[grabArrays.length-1];
> + RemoveRandomWithObject[] newArray = new
RemoveRandomWithObject[grabArraysLength > 1 ? grabArraysLength-1 : 0];
We have already checked grabArrays.length.
> if(x > 0)
> System.arraycopy(grabArrays, 0,
> newArray, 0, x);
> - if(x < grabArrays.length-1)
> - System.arraycopy(grabArrays, x+1,
> newArray, x, grabArrays.length -
(x+1));
> + if(x < grabArraysLength-1)
> + System.arraycopy(grabArrays, x+1,
> newArray, x, grabArraysLength -
(x+1));
More bizarreness.
> grabArrays = newArray;
> }
> if(item == null) {
> @@ -141,7 +142,7 @@
> // Hmmm...
> excluded++;
> if(excluded > MAX_EXCLUDED) {
> - Logger.normal(this, "Too many
> sub-arrays are entirely excluded
on "+this+" length = "+grabArrays.length, new Exception("error"));
> + Logger.normal(this, "Too many
> sub-arrays are entirely excluded
on "+this+" length = "+grabArraysLength, new Exception("error"));
Just revert the whole commit, please?
> return null;
> }
> }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL:
<https://emu.freenetproject.org/pipermail/devl/attachments/20080414/49aef119/attachment.pgp>
