Hi,
On Wed, 4 Jun 2008, Pieter de Bie wrote:
> +static void export_marks(char * file)
Extra space after star.
> +{
> + unsigned int i;
> + uintmax_t mark;
> + struct object_decoration *deco = idnums.hash;
> + FILE *f;
> +
> + f = fopen(file, "w");
> + if (!f)
> + error("Unable to open marks file %s for writing", file);
> +
> + for (i = 0; i < idnums.size; ++i) {
> + deco++;
> + if (deco && deco->base && deco->base->type == 1) {
> + mark = (uint32_t *) deco-> decoration - (uint32_t *)NULL;
Why do you use uint32_t here, when you use uintmax_t to declare "mark"?
Also, there is an extra space after the closing paren.
Is "- (uint32_t *)NULL" needed?
> + fprintf(f, ":%" PRIuMAX " %s\n", mark, sha1_to_hex(deco->base->sha1));
Too long line.
If you already only use uint32_t, I think you do not need the (ugly)
PRIuMAX.
> +static void import_marks(char * input_file)
> +{
> + char line[512];
> + FILE *f = fopen(input_file, "r");
> + if (!f)
> + die("cannot read %s: %s", input_file, strerror(errno));
> +
> + while (fgets(line, sizeof(line), f)) {
> + uintmax_t mark;
> + char *end;
> + unsigned char sha1[20];
> + struct object *object;
> +
> + end = strchr(line, '\n');
> + if (line[0] != ':' || !end)
> + die("corrupt mark line: %s", line);
> + *end = 0;
> + mark = strtoumax(line + 1, &end, 10);
> + if (!mark || end == line + 1
> + || *end != ' ' || get_sha1(end + 1, sha1))
> + die("corrupt mark line: %s", line);
You do a bit too much with "end" for my liking. Better use two variables,
and spare the reader a (brief) "Huh?" moment.
> + object = parse_object(sha1);
> + if (!object)
> + die ("Could not read blob %s", sha1_to_hex(sha1));
> +
> + if (object->flags & SHOWN)
> + error("Object %s was already has a mark", sha1);
s/was //
> + add_decoration(&idnums, object, ((uint32_t *)NULL) + mark);
Better write (void *)mark.
> + char *export_filename, *import_filename;
> struct option options[] = {
> OPT_INTEGER(0, "progress", &progress,
> "show progress after <n> objects"),
> OPT_CALLBACK(0, "signed-tags", &signed_tag_mode, "mode",
> "select handling of signed tags",
> parse_opt_signed_tag_mode),
> + OPT_STRING(0, "export-marks", &export_filename, "FILE", "Dump marks to this file"),
> + OPT_STRING(0, "import-marks", &import_filename, "FILE", "Import marks from this file"),
Two long lines.
> OPT_END()
> };
>
> /* we handle encodings */
> git_config(git_default_config, NULL);
>
> +
> init_revisions(&revs, prefix);
Unnecessary change.
Other than that: ACK.
Thanks,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
