[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: coreutils tr cleanup for integer types etc.
From: |
Jim Meyering |
Subject: |
Re: coreutils tr cleanup for integer types etc. |
Date: |
Mon, 31 May 2004 20:40:18 +0200 |
Paul Eggert <address@hidden> wrote:
> I read coreutils CVS tr.c and noticed a lot of minor gotchas
> with integer types, some of which lead to incorrect behavior.
> For example, here's the behavior with x86 coreutils 5.2.1 tr:
> each command behaves incorrectly.
>
> $ echo a | tr a '[x*][y*2147483646][y*2147483646][y*4]'
> x
> $ echo abcd | tr abc '[b*\9]'
> bbbd
> $ echo abcd | tr abc '[b*0]'
> tr: invalid repeat count `0' in [c*n] construct
> $ echo abcd | tr -c '[a*65536]\n' '[b*]'
> tr: ../../coreutils-5.2.1/src/tr.c:1942: main: Assertion `get_next (s2,
> ((void *)0)) == -1 || truncate_set1' failed.
> Aborted
>
> Here is a patch.
>
> 2004-05-29 Paul Eggert <address@hidden>
>
> tr cleanup, mostly having to do with integer type ranges.
> Remove all casts.
Thanks a lot! It's certainly humbling to find latent bugs
in code I thought was solid. I've applied your patch.
I know I'm looking the gift horse in the mouth again,
but if you can find the energy next time, would you please
try to separate the clean-up-style changes from those that
actually fix bugs?
FYI, I've had to apply an additional change to work around
what looks like a bug in HP's /bin/cc compiler.
2004-05-30 Jim Meyering <address@hidden>
Work around HPUX /bin/cc compiler bug that is exposed, now that
sets are arrays of type `bool'. More details here:
http://lists.gnu.org/archive/html/bug-gnulib/2004-05/msg00094.html
FIXME: verify that the above URL points to the right message
* src/tr.c (card_of_complement): Use cleaner `sizeof in_set'
rather than `N_CHARS * sizeof(in_set[0])'. Using HPUX's /bin/cc
(aC++/ANSI C B3910B A.05.55 [Dec 04 2003]) on an ia64-hp-hpux11.22
system, those two expressions are not the same (256 vs. 1024).
The effect of this problem was that `tr -c x y' would fail:
tr: when not truncating set1, string2 must be non-empty
(set_initialize): Remove unnecessary initialization of the `in_set'
buffer; that initialization triggered the same compiler bug as above.
--- tr.c-orig 2004-05-31 20:32:00.222507174 +0200
+++ tr.c 2004-05-31 20:33:07.150705574 +0200
@@ -1200,7 +1200,7 @@ card_of_complement (struct Spec_list *s)
int cardinality = N_CHARS;
bool in_set[N_CHARS];
- memset (in_set, 0, N_CHARS * sizeof (in_set[0]));
+ memset (in_set, 0, sizeof in_set);
s->state = BEGIN_STATE;
while ((c = get_next (s, NULL)) != -1)
{
@@ -1664,10 +1664,11 @@ read_and_xlate (char *buf, size_t size)
return bytes_read;
}
-/* Initialize a boolean membership set IN_SET with the character
+/* Initialize a boolean membership set, IN_SET, with the character
values obtained by traversing the linked list of constructs S
- using the function `get_next'. If COMPLEMENT_THIS_SET is
- true the resulting set is complemented. */
+ using the function `get_next'. IN_SET is expected to have been
+ initialized to all zeros by the caller. If COMPLEMENT_THIS_SET
+ is true the resulting set is complemented. */
static void
set_initialize (struct Spec_list *s, bool complement_this_set, bool *in_set)
@@ -1675,7 +1676,6 @@ set_initialize (struct Spec_list *s, boo
int c;
size_t i;
- memset (in_set, 0, N_CHARS * sizeof (in_set[0]));
s->state = BEGIN_STATE;
while ((c = get_next (s, NULL)) != -1)
in_set[c] = true;