Re: [PATCH 2/2] remote-curl.c: fix rpc_out()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



If I understand the change right, the only thing that really matters
here is removing the extra ';' in the if (max < avail) condition.

That bug was the only reason why rpc->len < rpc->pos, and is thus
the only reason why avail would "go negative" (which it can't do
since its unsigned, it just wrapped around to a massive value).

Tay Ray Chuan <rctay89@xxxxxxxxx> wrote:
> Use comparisons between rpc->len and rpc->pos, rather than computing
> their difference. This avoids potential errors when this value is
> negative and we access it.

I don't see this as being very relevant here.
 
> Use an int to store the return value from packet_read_line(), instead
> of a size_t.

Why?  The code is actually harder to follow.  packet_read_line does
not return a negative value.

> Handle the errorneous condition where rpc->pos exceeds rpc->len by
> printing a message and aborting the transfer (return 0).

This condition should be impossible.  It only occurred because of
the bug you were trying to fix, which is this one next item:
 
> Remove extraneous semicolon (';') at the end of the if statement, that
> prevented code in its block from executing.

Right.  This bug is really bad and should be fixed.  But the message
above make this some little after-thought and doesn't explain what
was wrong with this being here.
 
> ---
> 
>   Users might experience issues when pushing with chunked encoding when
>   size_t avail is negative.

Shouldn't this be in the commit message?  Or something about how
pushing with chunked encoding was broken if the push was larger
than the default buffer size of 1 MiB?
 
-- 
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]