[gold patch] Incremental 15/18: Add --incremental-base option.

Cary Coutant ccoutant@google.com
Tue May 24 22:20:00 GMT 2011


>> @@ -861,6 +861,7 @@ Incremental_inputs::report_command_line(int argc, const char* const* argv)
>>         || strcmp(argv[i], "--incremental-changed") == 0
>>         || strcmp(argv[i], "--incremental-unchanged") == 0
>>         || strcmp(argv[i], "--incremental-unknown") == 0
>> +       || is_prefix_of("--incremental-base=", argv[i])
>>         || is_prefix_of("--debug=", argv[i]))
>>          continue;
>
> Will this work correctly if the user does
>    ld --incremental-base old-file
> ?  That is, if they pass the argument as a separate argv entry, rather
> than using =?  I guess the same question arises for --debug.

No, that's a known limitation. I plan to overhaul the command-line
comparison, and was intending to fix that issue then. I'd like to
store each argument as a separate string in the string table (where
many of the filename arguments probably already exist), and have an
array of pointers to the command-line options. That should reduce the
storage space required and eliminate the escaping of quotes.

Well, it's fairly trivial to go ahead and fix that now. How does the
following look, added immediately after the above if statement?

      if (strcmp(argv[i], "--incremental-base") == 0
	  || strcmp(argv[i], "--debug") == 0)
	{
	  // When these options are used without the '=', skip the
	  // following parameter as well.
	  ++i;
	  continue;
	}

>> +  // If the base file and the output file are different, open a
>> +  // new output file and copy the contents from the base file.
>> +  if (use_base_file)
>> +    {
>> +      unsigned char* base = this->base_;
>> +      this->open(s.st_size);
>> +      memcpy(this->base_, base, s.st_size);
>> +      ::munmap(base, s.st_size);
>> +      ::close(o);
>> +    }
>
> You're going to some mild contortions to map in the base file, and then
> you are throwing away the mapping.  If you are going to stick with this
> copying approach, then I think it would be simpler to just map the
> output file as for a non-incremental link, and then open() the base file
> and read() the contents into the mapping you just created.  That will be
> just as efficient--it's a nice sequential read--and will save you from
> running out of memory when you in effect map the output file twice.

Argh, good point. I'll rework this.

-cary



More information about the Binutils mailing list