On 17-2-2016 11:26, Ivan Arabadzhiev intelrullz@...
[Firebird-Java] wrote:

> Thanks for responding :) I haven't really been through the FB code so I
> was hoping when you get the chance you'd lend a hand - chances are I'll
> need a few days just to figure out what I'm reading. The only part I'm
> sure about is that the length argument is 2-bytes wide because it is
> explicitly stated in the README and in the discussion for the .Net
> provider. Either way, I'm hoping any change would keep some different
> part of the code from malfunctioning, rather than break the restore.

I think one of the problems is that Firebird itself doesn't really
distinguish between "string" and byte arrays, while the code in Jaybird
does; so maybe excluding StringSpb is good enough.

Services are my least favorite part of Firebird, so I usually need to
dive into it as well :). I am not going to have enough time today, so it
will be on Saturday when I can take a real in-depth look.

> On that note, I also don't like assuming an order of the elements in the
> buffer, so that will have to come 'any day now'.

Firebird service requests only return the elements you asked for, so you
should be safe there, although it would be better to handle it as part
of the switch that also handles isc_info_truncated, isc_info_svc_line
and isc_info_end.

But it looks like you are skipping some of the information in the buffer
(eg some of the verbose output) by doing a service.getServiceInfo in the
middle of the loop when processing isc_info_svc_stdin (here:
https://github.com/ls4f/jaybird/blob/008f447b885ca9d7ffbbc926eee25b6f1262a312/src/main/org/firebirdsql/management/FBStreamingBackupManager.java#L409
).

But I might be mistaken, as I don't yet know exactly what Firebird sends
and expects here.

Also note that calls like
buffer = service.getServiceInfo(tempSPB, infoSRB, actuallyReadBytes);

should be

buffer = service.getServiceInfo(tempSPB, infoSRB, bufferSize);

That argument is the buffer size 'allocated' at the client to get
information from the server, not the amount of data you are sending to
the server.

> Is for available(), I wasn't really happy with it either (which is why I
> didn't start with it). The change in question is
> https://github.com/ls4f/jaybird/commit/008f447b885ca9d7ffbbc926eee25b6f1262a312
> . I think I could add some 'random' element to the bytes requested and
> try to beat some sense to InputStream that way.

I think you should be using the following:

if (actuallyReadBytes == -1) {
infoSRB = service.createServiceRequestBuffer();
infoSRB.addArgument(isc_info_svc_line);
}

That is: you have read the entire stream, so you are done with sending.

actuallyReadBytes can never be 0, and -1 means that end of stream has
been reached. I am not sure though if Firebird requires an explicit
notification of that though.

> You will have a scanned license agreement by the end of the week but for
> now I want to spend any possible time on polishing the code.

No problem, thanks. When polishing, could you also format the code using
the (Eclipse or IntelliJ) formatting definition in build/formatting/?

If you do, only apply it to the modified files (existing files are not
always consistently formatted, and I prefer not to change formatting
unless necessary).

> However,
> since you mention it - I'm guessing I should copy the initial comment
> from the other files, but do I copy it with the names or do I fill
> myself in (never really wanted to document my existence , but I am
> willing to take the blame for any possible headaches caused by my code :))

For the file-level license comment, you can copy the one from
FBBackupManager. For the class level javadoc, you could copy the one
from FBBackupManager. If you want you can add another @author tag for
yourself, but it is up to you if you want to include that. If you don't
want to get credit in the source code, then just leave it off (we can
always git blame ;).

Mark
--
Mark Rotteveel