Re: [PATCH] builtin-fast-export: Add importing and exporting of revision marks

On 5 jun 2008, at 02:00, Johannes Schindelin wrote:

Hi,

On Wed, 4 Jun 2008, Pieter de Bie wrote:

quoted

+{
+	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?

I changed the uintmax_t to to a uint32_t. If I remove the "- (uint32_t  
*)NULL",
it won't return the same marks. The same is done in get_object_mark().

quoted

+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.

Right. I copied this code from fast-export.c. I changed it to two  
variables now.

quoted

+		add_decoration(&idnums, object, ((uint32_t *)NULL) + mark);

Better write (void *)mark.

That won't return the same result, as pointer addition goes with 4  
bytes. The
same thing is done in mark_object().

I will send an updated patch.

- Pieter