flowpowered / network

Networking library for the Flow collection.
https://flow.github.io/network
MIT License
26 stars 18 forks source link

readVarInt() missing bytes increment #7

Closed deathcap closed 10 years ago

deathcap commented 10 years ago

https://github.com/flow/flow-networking/blob/master/src/main/java/com/flowpowered/networking/util/ByteBufUtils.java


    public static int readVarInt(ByteBuf buf) throws IOException {
        int out = 0;
        int bytes = 0;
        byte in;
        while (true) {
            in = buf.readByte();
            out |= (in & 0x7F) << (bytes * 7);
            if (bytes > 32) {
                throw new IOException("Attempt to read int bigger than allowed for a varint!");
            }
            if ((in & 0x80) != 0x80) {
                break;
            }
        }
        return out;
    }

The bytes variable is constant zero. Shouldn't it be incremented in the loop?

kitskub commented 10 years ago

Yes probably. Thanks for the heads up.

SpaceManiac commented 10 years ago

While you're looking at it, "bytes > 32" seems like a really big allowance for something that's supposed to be preventing overflow. Maybe 4 (28 bits) is more appropriate? I've also seen 5 used in some places.

kitskub commented 10 years ago

@SpaceManiac 4 bytes is actually all an int can hold anyways. So anything larger than that would need a BigInteger.

kitskub commented 10 years ago

Thanks @deathcap