RE: resolver //Was: [PATCH 3/7] Debug output to show both IP and port # in native b.o., a few little cosmetic improvements for consistency

["Lavrentiev, Anton (NIH/NLM/NCBI) [C]" <lavr@ncbi.nlm.nih.gov>]

Hi Corinna,

> Other than that, the remaining patches look good, except, adding a short
> description what patch 7 does to the commit message would be great for
> later readers of the git log.

I resubmitted the patches with a little improvement and a better description
to the #7 (now #5) as requested.

While doing the code review afresh in there, I noticed a few more problems:

1.
minires-os-if.c on line 262 does this:
    262         ptr = NULL;
    263         break;

and then a bit later this:
    290   len = ptr - AnsPtr;

which makes the return value "len" to be a total nonsense (I think it should
return -1 in this case, but it's debatable).

2.
Also, "ptr" in the cygwin_query() function is not checked for buffer overrun
in the "done:" section of the code (after line 291), which is not good IMO.

3.
Lastly, at other places where "ptr" is checked for overrun (notably, in write_record()),
it can leave garbage in the unfilled portion of the answer buffer (because it still
advances "ptr" properly, to calculate the final "would-be" size of the response):
so if the return value is greater than the passed "AnsLength", the user cannot assume
that at least all AnsLength bytes were filled correctly.  They cannot actually assume
any "boundary" where the "Ans" buffer was actually stopped being updated.

Maybe "Ans" should be cleared upon entry?... But that would mean double-write of
the buffer in most of the use-cases (where no overflow actually occurs because of
an adequate size of the buffer).

Anton Lavrentiev
Contractor NIH/NLM/NCBI