On Sat, Nov 25, 2017 at 4:47 PM, Max Kirillov <max@xxxxxxxxxx> wrote: > Thanks for the review. I saw only reaction of the Jeff in > the original thread and though that it is ok otherwise. I'm > fixing the things you mentioned. The commentary (in which you talked about restoring the patch and squashing) seemed to imply that this had been posted somewhere before, but it wasn't marked as "v2" (or whatever attempt) and lacked a URL pointing at the previous attempt, so it was difficult to judge. > On Thu, Nov 23, 2017 at 08:30:39PM -0500, Eric Sunshine wrote: >>> +static ssize_t read_request_fix_len(int fd, size_t req_len, unsigned char **out) >> >> Wrong data type: s/size_t req_len/ssize_t req_len/ > > Passing negative value to the function makes no sense. I > could add explicit type cast to make it clear. It should be > safe as site_t's range is bigger, and overflown > CONTENT_LENGTH results in die() at parsing (I have a test > which verifies it) A concern with requesting size_t bytes is that, if it does read all bytes, that value can't necessarily be represented by the ssize_t returned from the function. Where would the cast be placed that you suggest? How do other git functions deal with this sort of situation?