db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bryan Pendleton (JIRA)" <j...@apache.org>
Subject [jira] [Created] (DERBY-6997) Confusing code in NetworkServerControlImpl could be improved
Date Sat, 05 May 2018 15:31:00 GMT
Bryan Pendleton created DERBY-6997:
--------------------------------------

             Summary: Confusing code in NetworkServerControlImpl could be improved
                 Key: DERBY-6997
                 URL: https://issues.apache.org/jira/browse/DERBY-6997
             Project: Derby
          Issue Type: Improvement
          Components: Network Server
    Affects Versions: 10.14.2.0
            Reporter: Bryan Pendleton


As was discussed on the derby-dev list here: http://mail-archives.apache.org/mod_mbox/db-derby-dev/201802.mbox/%3CCANi-yg8ru8BLNVxW0v6TT_ndPa8Y7_MD6zpgjiyQ6MQoT4Ca9g%40mail.gmail.com%3E

it seems like there is some confusing shifting-and-masking code in NetworkServerControlImpl
that could be improved:

{code:java}
    private void writeShort(int value) throws Exception
    {
        try {
            commandOs.writeByte((byte)((value & 0xf0) >> 8 ));
            commandOs.writeByte((byte)(value & 0x0f));
        }
        catch (IOException e)
        {
            clientSocketError(e);
        }
    }
{code}
I'm not quite sure what this code is doing.

It seems to be mixing some combination of 8-bit shifting
and 4-bit value masking.

I think it might actually be losing/destroying 4 bits of
the 16-bit value (the high 4 bytes in the low byte of
the short).

For example if you call writeShort(23), it emits 0x00, 0x07,
when I think it should emit 0x00,0x17.

I suspect the code should read:
{code:java}
    private void writeShort(int value) throws Exception
    {
        try {
            commandOs.writeByte((byte)((value & 0xff00) >> 8 ));
            commandOs.writeByte((byte)(value & 0x00ff));
        }
        catch (IOException e)
        {
            clientSocketError(e);
        }
    }
{code}
or perhaps
{code:java}
    private void writeShort(int value) throws Exception
    {
        try {
            commandOs.writeByte((byte)((value >> 8 ));
            commandOs.writeByte((byte)value);
        }
        catch (IOException e)
        {
            clientSocketError(e);
        }
    }
{code}
or some such, but really I don't understand it at all.

And, very close to this code is a very similar method:
{code:java}
    private void writeByte(int value) throws Exception
    {
        try {
            commandOs.writeByte((byte)(value & 0x0f));
        }
        catch (IOException e)
        {
            clientSocketError(e);
        }
    }
{code}
which I think might be damaged for any value in the
range 17-255? It seems like it should probably be:
{code:java}
    private void writeByte(int value) throws Exception
    {
        try {
            commandOs.writeByte((byte)(value & 0xff));
        }
        catch (IOException e)
        {
            clientSocketError(e);
        }
    }
{code}
or maybe just:
{code:java}
    private void writeByte(int value) throws Exception
    {
        try {
            commandOs.writeByte((byte)value);
        }
        catch (IOException e)
        {
            clientSocketError(e);
        }
    }
{code}




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message