Re: [PATCH] object-info: support for retrieving object info

[Junio C Hamano <gitster@pobox.com>]

Bruno Albuquerque <bga@google.com> writes:

> +int cap_object_info(struct repository *r, struct strvec *keys,
> +		    struct packet_reader *request)
> +{
> +	struct packet_writer writer;
> +	packet_writer_init(&writer, 1);

This triggers -Wdeclaration-after-statement below.  Move it down.

> +	int parsed_header;
> +	struct requested_info info;
> +

Puzzling blank line here.  There does not seem to be a good reason
why 'parsed_header' bit plus 'info' pair together closely than to
the other 'oid_str_list' variable, so it does not make much sense as
a grouping aid.

> +	struct string_list oid_str_list = STRING_LIST_INIT_DUP;
> +

Here, just before "parsed_header = 0;" after the blank line that
separates the decls and the statements, is a good place to say
"packet_writer_init()".  Also it may make more sense to give initial
value to parsed_header where it is declared.

> +	parsed_header = 0;
> +	while (packet_reader_read(request) == PACKET_READ_NORMAL) {
> +		if (!strcmp("size", request->line)) {
> +			info.size = 1;
> +			continue;

And upon further inspection, nobody seems to use parsed_header at
all.  Let's lose it.

Perhaps this minimum fix-up squashed in?

Next time, perhaps try "make DEVELOPER=YesPlease test" to catch
possible problems like these early?

Thanks.


 protocol-caps.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git c/protocol-caps.c w/protocol-caps.c
index c15e397756..922bfeb905 100644
--- c/protocol-caps.c
+++ w/protocol-caps.c
@@ -78,13 +78,10 @@ int cap_object_info(struct repository *r, struct strvec *keys,
 		    struct packet_reader *request)
 {
 	struct packet_writer writer;
-	packet_writer_init(&writer, 1);
-	int parsed_header;
 	struct requested_info info;
-
 	struct string_list oid_str_list = STRING_LIST_INIT_DUP;
 
-	parsed_header = 0;
+	packet_writer_init(&writer, 1);
 	while (packet_reader_read(request) == PACKET_READ_NORMAL) {
 		if (!strcmp("size", request->line)) {
 			info.size = 1;