Re: crc

[Simon Josefsson]

Paul Eggert <address@hidden> writes:

> Simon Josefsson <address@hidden> writes:
>
>> I'm not sure whether the extract from RFC 1952 (a two line function)
>> warrant the big copyright comment, or whether borrowing that code is
>> OK.
>
> No, it's OK to borrow two lines of code.  However it would be polite
> to have a one- or two-line comment giving the reference.

The comment before the function include references and the copyright.
Should I make it shorter, i.e.:

/*
 * The following function was extracted from RFC 1952 by Simon
 * Josefsson.  It was modified to avoid initial and final XOR, to use
 * size_t for the buffer length, and to use the const keyword.
 */

instead of the current:

/*
 * The following function was extracted from RFC 1952 by Simon
 * Josefsson, and modified to avoid initial and final XOR and to use
 * size_t for the buffer length.  The original copyright and notice
 * from the document follows:
 *
 *    Copyright (c) 1996 L. Peter Deutsch
 *
 *    Permission is granted to copy and distribute this document for
 *    any purpose and without charge, including translations into
 *    other languages and incorporation into compilations, provided
 *    that the copyright notice and this notice are preserved, and
 *    that any substantive changes or deletions from the original are
 *    clearly marked.
 *
 * The copyright on RFCs, and consequently the function below, are
 * supposedly also retroactively claimed by the Internet Society
 * (according to address@hidden), with the following
 * copyright notice:
 *
 *    Copyright (C) The Internet Society.  All Rights Reserved.
 *
 *    This document and translations of it may be copied and furnished
 *    to others, and derivative works that comment on or otherwise
 *    explain it or assist in its implementation may be prepared,
 *    copied, published and distributed, in whole or in part, without
 *    restriction of any kind, provided that the above copyright
 *    notice and this paragraph are included on all such copies and
 *    derivative works.  However, this document itself may not be
 *    modified in any way, such as by removing the copyright notice or
 *    references to the Internet Society or other Internet
 *    organizations, except as needed for the purpose of developing
 *    Internet standards in which case the procedures for copyrights
 *    defined in the Internet Standards process must be followed, or
 *    as required to translate it into languages other than English.
 *
 *    The limited permissions granted above are perpetual and will not be
 *    revoked by the Internet Society or its successors or assigns.
 *
 *    This document and the information contained herein is provided
 *    on an "AS IS" basis and THE INTERNET SOCIETY AND THE INTERNET
 *    ENGINEERING TASK FORCE DISCLAIMS ALL WARRANTIES, EXPRESS OR
 *    IMPLIED, INCLUDING BUT NOT LIMITED TO ANY WARRANTY THAT THE USE
 *    OF THE INFORMATION HEREIN WILL NOT INFRINGE ANY RIGHTS OR ANY
 *    IMPLIED WARRANTIES OF MERCHANTABILITY OR FITNESS FOR A
 *    PARTICULAR PURPOSE.
 *
 */


>> +static unsigned long crc32_table[256] = {
>> +  0x00000000, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
>
> The usual GNU style is to say 'unsigned long int' rather than 'unsigned long'.

Changed.

> But shouldn't you be using uint32_t rather than unsigned long int,
> both here and elsewhere?  AC_REQUIRE([gl_AC_TYPE_UINT32_T]), and then
> put this in your C file:
>
> ##if HAVE_INTTYPES_H
> # include <inttypes.h>
> #endif
> #if HAVE_STDINT_H
> # include <stdint.h>
> #endif
>
> and you can use uint32_t.  I suggest using it in both the .h and the
> .c file, since it makes things clearer.

Yes, makes sense.  I made the module depend on stdint, and put the CPP
stuff in crc.h.  I'll post the updated module in a separate e-mail.

Thanks,
Simon