Merge remote-tracking branch 'origin/master' into no-pkt-pack

Author: nelhage

deps/http-parser/CMakeLists.txt

@@ -1,3 +1,5 @@
 FILE(GLOB SRC_HTTP "*.c" "*.h")
 
 ADD_LIBRARY(http-parser OBJECT ${SRC_HTTP})
+
+ENABLE_WARNINGS(implicit-fallthrough=1)

src/config_file.c

@@ -876,10 +876,8 @@ static char *escape_value(const char *ptr)
 		ptr++;
 	}
 
-	if (git_buf_oom(&buf)) {
-		git_buf_dispose(&buf);
+	if (git_buf_oom(&buf))
 		return NULL;
-	}
 
 	return git_buf_detach(&buf);
 }
@@ -1022,33 +1020,33 @@ static int parse_conditional_include(git_config_parser *reader,
 static int read_on_variable(
 	git_config_parser *reader,
 	const char *current_section,
-	char *var_name,
-	char *var_value,
+	const char *var_name,
+	const char *var_value,
 	const char *line,
 	size_t line_len,
 	void *data)
 {
 	diskfile_parse_state *parse_data = (diskfile_parse_state *)data;
 	git_buf buf = GIT_BUF_INIT;
 	git_config_entry *entry;
+	const char *c;
 	int result = 0;
 
 	GIT_UNUSED(line);
 	GIT_UNUSED(line_len);
 
-	git__strtolower(var_name);
-	git_buf_printf(&buf, "%s.%s", current_section, var_name);
-	git__free(var_name);
+	git_buf_puts(&buf, current_section);
+	git_buf_putc(&buf, '.');
+	for (c = var_name; *c; c++)
+		git_buf_putc(&buf, git__tolower(*c));
 
-	if (git_buf_oom(&buf)) {
-		git__free(var_value);
+	if (git_buf_oom(&buf))
 		return -1;
-	}
 
 	entry = git__calloc(1, sizeof(git_config_entry));
 	GITERR_CHECK_ALLOC(entry);
 	entry->name = git_buf_detach(&buf);
-	entry->value = var_value;
+	entry->value = var_value ? git__strdup(var_value) : NULL;
 	entry->level = parse_data->level;
 	entry->include_depth = parse_data->depth;
 
@@ -1065,7 +1063,6 @@ static int read_on_variable(
 		result = parse_conditional_include(reader, parse_data,
 						   entry->name, entry->value);
 
-
 	return result;
 }
 
@@ -1249,8 +1246,8 @@ static int write_on_section(
 static int write_on_variable(
 	git_config_parser *reader,
 	const char *current_section,
-	char *var_name,
-	char *var_value,
+	const char *var_name,
+	const char *var_value,
 	const char *line,
 	size_t line_len,
 	void *data)
@@ -1279,9 +1276,6 @@ static int write_on_variable(
 	if (has_matched && write_data->preg != NULL)
 		has_matched = (regexec(write_data->preg, var_value, 0, NULL, 0) == 0);
 
-	git__free(var_name);
-	git__free(var_value);
-
 	/* If this isn't the name/value we're looking for, simply dump the
 	 * existing data back out and continue on.
 	 */

src/config_parse.c

@@ -404,22 +404,21 @@ static int parse_name(
 static int parse_variable(git_config_parser *reader, char **var_name, char **var_value)
 {
 	const char *value_start = NULL;
-	char *line;
-	int quote_count;
+	char *line = NULL, *name = NULL, *value = NULL;
+	int quote_count, error;
 	bool multiline;
 
+	*var_name = NULL;
+	*var_value = NULL;
+
 	git_parse_advance_ws(&reader->ctx);
 	line = git__strndup(reader->ctx.line, reader->ctx.line_len);
-	if (line == NULL)
-		return -1;
+	GITERR_CHECK_ALLOC(line);
 
 	quote_count = strip_comments(line, 0);
 
-	/* If there is no value, boolean true is assumed */
-	*var_value = NULL;
-
-	if (parse_name(var_name, &value_start, reader, line) < 0)
-		goto on_error;
+	if ((error = parse_name(&name, &value_start, reader, line)) < 0)
+		goto out;
 
 	/*
 	 * Now, let's try to parse the value
@@ -428,30 +427,34 @@ static int parse_variable(git_config_parser *reader, char **var_name, char **var
 		while (git__isspace(value_start[0]))
 			value_start++;
 
-		if (unescape_line(var_value, &multiline, value_start, 0) < 0)
-			goto on_error;
+		if ((error = unescape_line(&value, &multiline, value_start, 0)) < 0)
+			goto out;
 
 		if (multiline) {
 			git_buf multi_value = GIT_BUF_INIT;
-			git_buf_attach(&multi_value, *var_value, 0);
+			git_buf_attach(&multi_value, value, 0);
 
 			if (parse_multiline_variable(reader, &multi_value, quote_count) < 0 ||
-				git_buf_oom(&multi_value)) {
+			    git_buf_oom(&multi_value)) {
+				error = -1;
 				git_buf_dispose(&multi_value);
-				goto on_error;
+				goto out;
 			}
 
-			*var_value = git_buf_detach(&multi_value);
+			value = git_buf_detach(&multi_value);
 		}
 	}
 
-	git__free(line);
-	return 0;
+	*var_name = name;
+	*var_value = value;
+	name = NULL;
+	value = NULL;
 
-on_error:
-	git__free(*var_name);
+out:
+	git__free(name);
+	git__free(value);
 	git__free(line);
-	return -1;
+	return error;
 }
 
 int git_config_parse(
@@ -463,7 +466,7 @@ int git_config_parse(
 	void *data)
 {
 	git_parse_ctx *ctx;
-	char *current_section = NULL, *var_name, *var_value;
+	char *current_section = NULL, *var_name = NULL, *var_value = NULL;
 	int result = 0;
 
 	ctx = &parser->ctx;
@@ -508,7 +511,10 @@ int git_config_parse(
 		default: /* assume variable declaration */
 			if ((result = parse_variable(parser, &var_name, &var_value)) == 0 && on_variable) {
 				result = on_variable(parser, current_section, var_name, var_value, line_start, line_len, data);
+				git__free(var_name);
+				git__free(var_value);
 			}
+
 			break;
 		}
 

src/config_parse.h

@@ -36,8 +36,8 @@ typedef int (*git_config_parser_section_cb)(
 typedef int (*git_config_parser_variable_cb)(
 	git_config_parser *parser,
 	const char *current_section,
-	char *var_name,
-	char *var_value,
+	const char *var_name,
+	const char *var_value,
 	const char *line,
 	size_t line_len,
 	void *data);

src/revwalk.c

@@ -455,7 +455,7 @@ static int get_revision(git_commit_list_node **out, git_revwalk *walk, git_commi
 	 */
 	if (!walk->limited) {
 		if ((error = add_parents_to_list(walk, commit, list)) < 0)
-		return error;
+			return error;
 	}
 
 	*out = commit;
@@ -678,7 +678,7 @@ void git_revwalk_sorting(git_revwalk *walk, unsigned int sort_mode)
 		walk->enqueue = &revwalk_enqueue_unsorted;
 	}
 
-	if (sort_mode != GIT_SORT_NONE)
+	if (walk->sorting != GIT_SORT_NONE)
 		walk->limited = 1;
 }
 
@@ -737,6 +737,7 @@ void git_revwalk_reset(git_revwalk *walk)
 	walk->walking = 0;
 	walk->limited = 0;
 	walk->did_push = walk->did_hide = 0;
+	walk->sorting = GIT_SORT_NONE;
 }
 
 int git_revwalk_add_hide_cb(

src/transports/smart.c

@@ -167,8 +167,10 @@ int git_smart__update_heads(transport_smart *t, git_vector *symrefs)
 			git_vector_foreach(symrefs, j, spec) {
 				git_buf_clear(&buf);
 				if (git_refspec_src_matches(spec, ref->head.name) &&
-				    !(error = git_refspec_transform(&buf, spec, ref->head.name)))
+				    !(error = git_refspec_transform(&buf, spec, ref->head.name))) {
+					git__free(ref->head.symref_target);
 					ref->head.symref_target = git_buf_detach(&buf);
+				}
 			}
 
 			git_buf_dispose(&buf);
@@ -266,14 +268,21 @@ static int git_smart__connect(
 	/* We now have loaded the refs. */
 	t->have_refs = 1;
 
-	first = (git_pkt_ref *)git_vector_get(&t->refs, 0);
+	pkt = (git_pkt *)git_vector_get(&t->refs, 0);
+	if (pkt && GIT_PKT_REF != pkt->type) {
+		giterr_set(GITERR_NET, "invalid response");
+		return -1;
+	}
+	first = (git_pkt_ref *)pkt;
 
 	if ((error = git_vector_init(&symrefs, 1, NULL)) < 0)
 		return error;
 
 	/* Detect capabilities */
-	if (git_smart__detect_caps(first, &t->caps, &symrefs) < 0)
+	if (git_smart__detect_caps(first, &t->caps, &symrefs) < 0) {
+		free_symrefs(&symrefs);
 		return -1;
+	}
 
 	/* If the only ref in the list is capabilities^{} with OID_ZERO, remove it */
 	if (1 == t->refs.length && !strcmp(first->head.name, "capabilities^{}") &&

src/transports/smart_pkt.c

@@ -203,6 +203,11 @@ static int ref_pkt(git_pkt **out, const char *line, size_t len)
 	git_pkt_ref *pkt;
 	size_t alloclen;
 
+	if (len < GIT_OID_HEXSZ + 1) {
+		giterr_set(GITERR_NET, "error parsing pkt-line");
+		return -1;
+	}
+
 	pkt = git__malloc(sizeof(git_pkt_ref));
 	GITERR_CHECK_ALLOC(pkt);
 
@@ -470,6 +475,9 @@ int git_pkt_parse_line(
 
 void git_pkt_free(git_pkt *pkt)
 {
+	if (pkt == NULL) {
+		return;
+	}
 	if (pkt->type == GIT_PKT_REF) {
 		git_pkt_ref *p = (git_pkt_ref *) pkt;
 		git__free(p->head.name);

src/transports/smart_protocol.c

@@ -70,6 +70,12 @@ int git_smart__store_refs(transport_smart *t, int flushes)
 			return -1;
 		}
 
+		if (pkt->type == GIT_PKT_PACK) {
+			giterr_set(GITERR_NET, "unexpected packfile");
+			git__free(pkt);
+			return -1;
+		}
+
 		if (pkt->type != GIT_PKT_FLUSH && git_vector_insert(refs, pkt) < 0)
 			return -1;
 
@@ -317,27 +323,30 @@ static int fetch_setup_walk(git_revwalk **out, git_repository *repo)
 static int wait_while_ack(gitno_buffer *buf)
 {
 	int error;
-	git_pkt_ack *pkt = NULL;
+	git_pkt *pkt = NULL;
+	git_pkt_ack *ack = NULL;
 
 	while (1) {
-		git__free(pkt);
+		git_pkt_free(pkt);
 
-		if ((error = recv_pkt((git_pkt **)&pkt, NULL, buf)) < 0)
+		if ((error = recv_pkt(&pkt, NULL, buf)) < 0)
 			return error;
 
 		if (pkt->type == GIT_PKT_NAK)
 			break;
+		if (pkt->type != GIT_PKT_ACK)
+			continue;
 
-		if (pkt->type == GIT_PKT_ACK &&
-		    (pkt->status != GIT_ACK_CONTINUE &&
-		     pkt->status != GIT_ACK_COMMON &&
-		     pkt->status != GIT_ACK_READY)) {
-			git__free(pkt);
-			return 0;
+		ack = (git_pkt_ack*)pkt;
+
+		if (ack->status != GIT_ACK_CONTINUE &&
+		    ack->status != GIT_ACK_COMMON &&
+		    ack->status != GIT_ACK_READY) {
+			break;
 		}
 	}
 
-	git__free(pkt);
+	git_pkt_free(pkt);
 	return 0;
 }
 
@@ -615,7 +624,8 @@ int git_smart__download_pack(
 			}
 		}
 
-		git__free(pkt);
+		git_pkt_free(pkt);
+
 		if (error < 0)
 			goto done;
 

tests/revwalk/basic.c

@@ -197,6 +197,31 @@ void test_revwalk_basic__push_head(void)
 	cl_assert_equal_i(i, 7);
 }
 
+void test_revwalk_basic__sorted_after_reset(void)
+{
+	int i = 0;
+	git_oid oid;
+
+	revwalk_basic_setup_walk(NULL);
+
+	git_oid_fromstr(&oid, commit_head);
+
+	/* push, sort, and test the walk */
+	cl_git_pass(git_revwalk_push(_walk, &oid));
+	git_revwalk_sorting(_walk, GIT_SORT_TIME);
+
+	cl_git_pass(test_walk_only(_walk, commit_sorting_time, 2));
+
+	/* reset, push, and test again - we should see all entries */
+	git_revwalk_reset(_walk);
+	cl_git_pass(git_revwalk_push(_walk, &oid));
+
+	while (git_revwalk_next(&oid, _walk) == 0)
+		i++;
+
+	cl_assert_equal_i(i, commit_count);
+}
+
 void test_revwalk_basic__push_head_hide_ref(void)
 {
 	int i = 0;