Re: [PATCH v4 5/5] Calculate CRC64 over the .text area

[Ulf Samuelsson <binutils@emagii.com>]

On 2023-02-21 14:26, Nick Clifton wrote:
> Hi Ulf,
>
> Please try to follow the GNU Coding standards when writing
> new code:
>
>   https://www.gnu.org/prep/standards/
>
> Having a consistent style greatly improves the readability
> of the code.
>
I will run 'indent' on my files.

If I add code to other files, is it OK to run indent on those?

>
>
>> +      einfo (_("%P:%pS: warning: Only the first CRC polynome is 
>> used\n"),
>> +        NULL);
>
> Is there a particular reason why multiple CRC polynomes are not 
> supported ?

Declaring a polynome will result in the CRC being calculated for the 
declared area.
That seems to be enough to cover the needs.

There may be a need for multiple areas, but I do not see a need for
calculating CRC64-ECMA on one area and CRC64-ISO on another area.

If the user wants to calculate CRC for another purpose, they can add code
in their project, but calculating CRC over the text area is problematic 
today.

>
>
>> +  for (bfd_vma i = 0 ; i < 256 ; i++)
>
> You could just use an integer for this loop....
>
True, but bfd_vma is a 64-bit integer, and I thought that keeping
that for everything would keep things simple.
>> +  /*
>> +   * Get the '.text' section
>> +   * Is there a risk that CRC needs to be calculated on more than 
>> .text?
>> +   * We do not support that...
>
> You definitely should.
>
Is there an easy way to figure out which section a symbol is located in?

If not, my idea is to implement "DIGEST SECTION "<section>" to specify this.


> I think that I will save a full review of this code until the 
> formatting is
> fixed up.
>
Tried running the testsuite, but

$ make check-ld RUNTESTFLAGS='ld-scripts/script.exp'

results in failures for my tests.

Can I retrieve any detailed error messages?


Cheers
>   Nick
>