Re: [PATCH] builtin-fast-export: Add importing and exporting of revision marks
- From
- Pieter de Bie <pdebie@ai.rug.nl>
- Date
- Jun 5, 2008, 10:46 UTC
- Message-ID
- <BEF1F17D-6F0F-4F09-9CC4-B193B8907901@ai.rug.nl>
- In-Reply-To
- <alpine.DEB.1.00.0806050052390.21190@racer>
On 5 jun 2008, at 02:00, Johannes Schindelin wrote:
Show 25 quoted lines
> Hi,
>
> On Wed, 4 Jun 2008, Pieter de Bie wrote:
>
>> +{
>> + 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().
Show 25 quoted lines
>> +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.
>> + 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