Re: [PATCH] object-info: support for retrieving object info
- From
Bruno Albuquerque <bga@google.com>
- Date
- Apr 20, 2021, 23:43 UTC
- Message-ID
- <CAPeR6H4V2_bAHaU4SCC8uGorj1bQEKLX5LWtXFuoHNRpo=dYPg@mail.gmail.com>
- In-Reply-To
- <xmqqmttznqj6.fsf@gitster.g>
On Thu, Apr 15, 2021 at 3:15 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> > +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.Done.
Show 7 quoted lines
> > + 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.
Done.
Show 7 quoted lines
> > + 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.
Done.
Show 8 quoted lines
> > + 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.Done.
> Next time, perhaps try "make DEVELOPER=YesPlease test" to catch > possible problems like these early?
I did. But I was coding on a MacOS machine (which defaults to clang) and it looks like the warnings are not triggered at all in clang even with -Werror being explicitly set. I switched to a Linux machine and got the warnings.