Re: [PATCH] crc: New optimised slice-by-8 implementation

[Sam Russell]

> Unfortunately, 'indent' introduced tabs. In Gnulib, we indent with spaces,
> not tabs (except in Makefiles and ChangeLog). Can you please untabify:

Done.

> Also, in GNU coding style, when line breaking is needed within expressions,
> we do the line break before the operator, not after the operator. [1]
> This affects lib/crc.c lines 73..80, 102..103.

Done.

> In the ChangeLog entry, please use today's date, not the date when you
> started the commit, per GNU coding style [2].

Done.

> In the ChangeLog entry for the module, please write it as:

Done.

> I'll then deal with the cross-compiling situation. Will need to experiment
> a bit, since that's the first time, in Gnulib, that we have a built file
> generated by a C program (as opposed to a pre-installed tool).

Glad to be on the bleeding edge :)

On Sun, 27 Oct 2024 at 12:58, Bruno Haible <bruno@clisp.org> wrote:

Sam Russell wrote:
> I used Simon's indent trick on both files

Unfortunately, 'indent' introduced tabs. In Gnulib, we indent with spaces,
not tabs (except in Makefiles and ChangeLog). Can you please untabify:
  $ expand < lib/crc.c > lib/crc.cx && mv lib/crc.clib/crc.c
  $ expand < lib/crc-generate-table.c > lib/crc-generate-table.cx \
    && mv lib/crc-generate-table.cx lib/crc-generate-table.c

Also, in GNU coding style, when line breaking is needed within expressions,
we do the line break before the operator, not after the operator. [1]
This affects lib/crc.c lines 73..80, 102..103.

In the ChangeLog entry, please use today's date, not the date when you
started the commit, per GNU coding style [2].

In the ChangeLog entry for the module, please write it as:

        * modules/crc (Depends-on): Add endian.
        (Makefile.am): Build slice-by-8 tables from crc-generate-table.c.

Then, from my point of view, it will be good to commit (by Simon).

> The @CROSS_COMPILING@ flag doesn't seem to be getting set and I'm only
> seeing it used in one other place, removing this makes it work and matches
> the pattern in modules/uninorm/composition that you recommended earlier.

I'll then deal with the cross-compiling situation. Will need to experiment
a bit, since that's the first time, in Gnulib, that we have a built file
generated by a C program (as opposed to a pre-installed tool).

> > In the 'main' function, please add a 'return 0;' statement at the end.
> It's
> > needed for Oracle cc 12.6.
>
> Nice catch, technically not valid C if I'm not returning from an int main().

It is valid C, to have no 'return' statement in the main() function, since C99
(see ISO C 99 § 5.1.2.2.3). But that particular compiler has been neglected by
its vendor.

Bruno

[1] https://www.gnu.org/prep/standards/html_node/Formatting.html
[2] https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html

0001-crc-New-optimised-slice-by-8-implementation.patch
Description: Binary data