git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 05/10] builtin-fsck: move common object checking code to fsck.c

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 26, 2008, 09:19 UTC
Message-ID
<7vskzg6pmw.fsf@gitster.siamese.dyndns.org>
In-Reply-To
<12039765002397-git-send-email-mkoegler@auto.tuwien.ac.at>

Is this series an unadjusted resend or something? This particular one had funny interaction with your own d4fe07f (git-fsck: report missing author/commit line in a commit as an error) that is already in 'master', so I had to munge it by hand. It was not so pleasant (a large chunk of code was moved from builtin-fsck.c to fsck.c), but that is what the maintainer does, so it's Ok. But I'd like you to eyeball the result to see if it looks sane.

I'll push it out as part of 'pu'. The tip of your topic is 154a955 (receive-pack: use strict mode for unpacking objects) tonight:

	Side note: To find a tip of a topic yourself, look for "Merge
	mk/maint-parse-careful" in "git log --first-parent
	origin/next..origin/pu" output and find the latest one.

One thing I noticed was that parse_$type_buffer() family all take non-const void *buf pointers but one new caller you introduced takes "const void *data" and passes that pointer to them. I hated to, but ended up loose-casting it. You may want to make the function take non-const pointer, but I did not look very carefully.

By the way, while I was at it, many stylistic issues bugged me too much, so I ended up fixing them as well:

 * Trailing whitespaces; avoid them.
 * Indenting with SP not HT; don't.
 * Pointer to struct foo type is (struct foo *), not (struct foo*);
 * One SP each around comparison operator "==";
 * One SP around assignment operator "=";
 * One SP after "if", "while", "switch" and friends before "(";
 * No SP between function name and "(";
 * A function without parameter is "static void foo(void)", not
   "static void foo()";
 * Decl-after-statement; don't.
 * Multi-line comment is:
	/*
         * This is multi line comment
         * and this is its second line.
         */
   not
	/* This is multi line comment
           and this is its second */
 * If you cast, cast to the right type ;-)
	struct tree *item = (struct tree *) obj;
   not
	struct tree *item = (struct item*) obj;

Please do not make me do this again, as I do not have infinite amount of time. This plea is not only about your patch but applies to everybody.

I wanted to merge a few new commits to existing topics to 'next' and merge down a few well cooked topics to 'master', but ran out of time tonight. New stuff I received and looked at are all parked in 'pu' tonight.

Previous: Martin KoeglerNext: Martin Koegler
Message 11 of 13 in “add generic, type aware object chain walker”
  1. 01/10 add generic, type aware object chain walkerMartin Koegler, Feb 25, 2008
  2. 02/10 builtin-fsck: move away from object-refs to fsck_walkMartin Koegler, Feb 25, 2008
  3. 03/10 Remove unused object-ref codeMartin Koegler, Feb 25, 2008
  4. 04/10 builtin-fsck: reports missing parent commitsMartin Koegler, Feb 25, 2008
  5. 05/10 builtin-fsck: move common object checking code to fsck.cMartin Koegler, Feb 25, 2008
  6. 06/10 add common fsck error printing functionMartin Koegler, Feb 25, 2008
  7. 07/10 unpack-object: cache for non written objectsMartin Koegler, Feb 25, 2008
  8. 08/10 unpack-objects: prevent writing of inconsistent objectsMartin Koegler, Feb 25, 2008
  9. 09/10 index-pack: introduce checking modeMartin Koegler, Feb 25, 2008
  10. 10/10 receive-pack: use strict mode for unpacking objectsMartin Koegler, Feb 25, 2008
  11. Junio C HamanoFeb 26, 2008
  12. Martin KoeglerFeb 26, 2008
  13. Junio C HamanoFeb 27, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.