Fix ByteArrayParameter::writeTo(SQLString& str) for negative sql::bytes::length

[Ebola-Chan-bot]

sql::bytes::length may be negative. That ByteArrayParameter::writeTo(SQLString& str) static_cast it directly to size_t causes unexpected result in this case.

[lawrinn]

Thank you for your pull request. Looks valid. But first - formalities.
In order to accept it, we need you write in the comment, that you are contributing your new code of the whole pull request, including one or several files that are either new files or modified ones, under the BSD-new license
Now couple of remarks.
is it intentional, that you don't use size() method that bytes have?
Would it be possible to add to the commit the testcase for your fix?

[Ebola-Chan-bot]

1. I'm contributing the new code of the whole pull request, including one or several files that are either new files or modified ones, under the BSD-new license.
2. Is this statement needed every PR, or is it only the first time?
3. I use sql::bytes::length simply because that's how the original code works. I want to introduce as little change as possible.
4. I could find this problem because the negative length was static_casted into a huge size_t causing subsequent code to determine that the string was too long and then fail:

void SetBytesAndExecute(sql::PreparedStatement* PreparedStatement, int32_t ParameterIndex)
{
	char NonConstChars[2];
	sql::bytes Bytes = sql::bytes(NonConstChars, 2);//The non-const char array selects a special overload that sets the length to be negative
	PreparedStatement->setBytes(ParameterIndex, &Bytes);
	PreparedStatement->execute();//string too long
}

I don't mind writing a test, but I find the directory structure under test quite complicated. Can you give me some knowledge of writing tests?

[lawrinn]

2. I am afraid each time. For server there is agreement that a contributor signs once, but server licensed differently
3. probably it's better to use size(), seems more right
4. Yeah, it's a bit unclear. I think it fits test/unit/classes/preparedstatement.(cpp|h) or test/unit/bugs/bugs.(cpp|h). Add method with the test to the class, and add TEST_CASE(<your_method_name>) in the header file in the top of class definition - you will see where. Thank you!

[Ebola-Chan-bot]

I've noticed that size() always returns a positive number. Do you agree that all lengths in the ByteArrayParameter.cpp should be changed to size()? I've further noticed that there are several other places in this file where length is used directly without considering that it might be negative.
I ask this because it's hard for me to write a test for every change in that file. I guess these changes only need one test as a representative, right?

[lawrinn]

For now it's better to change all length uses in ByteArrayClass to size(). It maybe worse considering to make it always make the copy of array or always refer the given array, ans so always have either positive or negative length. But that is another story. For now size() is the way to go

1 test should suffice - proving that array w/ negative length does not crash the program and works correctly.
In other places I will take care myself.