Re: [PATCH v4 0/5] Add support for CRC64 generation in linker

[Nick Clifton <nickc@redhat.com>]

Hi Ulf,

> Fourth patchset.
> Get rid of binaries in testsuite.

Thanks.

I do wonder if it would be possible to simplify some of the extensions
to the linker script syntax, in order to make it easier to use and to
allow for future extensions.  For example:

   DIGEST "algorithm" ( start_symbol,  end_symbol )

So "algorithm" could be "CRC" or "CRC32" or "CRC64" or "CRC64-WE" or
"CRC TABLE" or any of the other algorithms/features that you want to
support[1].  And more could be added in the future without having to
update the linker script syntax.

If the algorithm needs extra data apart from the start and end symbols
then this could be included in the name, eg:

   DIGEST "CRC64-POLY-0xDEADBEEFDEADBEEF" (ecc_start , ecc_end)

I would also suggest that functionality should not be restricted to
just the .text section, but that it should work for any section.  (It
is reasonable to restrict it to only working inside a single section
however).


> An algorithm typically needs
> <type> *init_<algo>_tab( <type> poly )
> <type> calc_<algo>_inv( const unsigned char *input_str, size_t num_bytes );
> <type> calc_<algo>    ( const unsigned char *input_str, size_t num_bytes );
> void lang_add_<algo>_syndrome(bool invert, bfd_vma poly)
> void lang_add_<algo>_table(void)

Since there are going to be several different versions of these functions,
and possibly more to come in the future, I think that it would be best to
put them into a new file. eg lddigest.c  This might also make it easier for
other linkers to make use of the code, should they decide to implement a
similar feature.


I am going to post some individual reviews of the patches as well, but I
wanted to give my overall impressions here.

Cheers
   Nick

[1] Well except for TIMESTAMP and DEBUG ON/OFF which are valid new
features in their own rights.