Re: [PATCH v2 08/25] upload-pack: use skip_prefix() instead of starts_with() when possible

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

 



Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---

The result is certainly easier to read.  "when possible" is not
quite a right way to look at it, though.  I think the patch does it
when it makes sense (i.e. when the result becomes easier to read),
which is much better ;-)

I initially thought that the name "arg" was too bland, but we can
think of these as pointing at an argument on each line that begins
with a command (e.g. "have", "shallow", etc.), and when viewed that
way, calling the second token on the line an "arg" makes sense.



>  upload-pack.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/upload-pack.c b/upload-pack.c
> index bfb7985..257ad48 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -276,7 +276,7 @@ static void create_pack_file(void)
>  	die("git upload-pack: %s", abort_msg);
>  }
>  
> -static int got_sha1(char *hex, unsigned char *sha1)
> +static int got_sha1(const char *hex, unsigned char *sha1)
>  {
>  	struct object *o;
>  	int we_knew_they_have = 0;
> @@ -382,6 +382,8 @@ static int get_common_commits(void)
>  
>  	for (;;) {
>  		char *line = packet_read_line(0, NULL);
> +		const char *arg;
> +
>  		reset_timeout();
>  
>  		if (!line) {
> @@ -403,8 +405,8 @@ static int get_common_commits(void)
>  			got_other = 0;
>  			continue;
>  		}
> -		if (starts_with(line, "have ")) {
> -			switch (got_sha1(line+5, sha1)) {
> +		if (skip_prefix(line, "have ", &arg)) {
> +			switch (got_sha1(arg, sha1)) {
>  			case -1: /* they have what we do not */
>  				got_other = 1;
>  				if (multi_ack && ok_to_give_up()) {
> @@ -616,14 +618,16 @@ static void receive_needs(void)
>  		const char *features;
>  		unsigned char sha1_buf[20];
>  		char *line = packet_read_line(0, NULL);
> +		const char *arg;
> +
>  		reset_timeout();
>  		if (!line)
>  			break;
>  
> -		if (starts_with(line, "shallow ")) {
> +		if (skip_prefix(line, "shallow ", &arg)) {
>  			unsigned char sha1[20];
>  			struct object *object;
> -			if (get_sha1_hex(line + 8, sha1))
> +			if (get_sha1_hex(arg, sha1))
>  				die("invalid shallow line: %s", line);
>  			object = parse_object(sha1);
>  			if (!object)
> @@ -636,19 +640,19 @@ static void receive_needs(void)
>  			}
>  			continue;
>  		}
> -		if (starts_with(line, "deepen ")) {
> +		if (skip_prefix(line, "deepen ", &arg)) {
>  			char *end;
> -			depth = strtol(line + 7, &end, 0);
> -			if (end == line + 7 || depth <= 0)
> +			depth = strtol(arg, &end, 0);
> +			if (end == arg || depth <= 0)
>  				die("Invalid deepen: %s", line);
>  			continue;
>  		}
> -		if (!starts_with(line, "want ") ||
> -		    get_sha1_hex(line+5, sha1_buf))
> +		if (!skip_prefix(line, "want ", &arg) ||
> +		    get_sha1_hex(arg, sha1_buf))
>  			die("git upload-pack: protocol error, "
>  			    "expected to get sha, not '%s'", line);
>  
> -		features = line + 45;
> +		features = arg + 40;
>  
>  		if (parse_feature_request(features, "multi_ack_detailed"))
>  			multi_ack = 2;
--
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]