On Thu, Apr 15, 2021 at 3:15 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > +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.
> > + 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.
> > + 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.
> > + 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.
