Bruno Haible wrote:
> Hi Jim,
>
> Jim Meyering wrote:
>> diff --git a/lib/trim.c b/lib/trim.c
>> index 1f4d0c1..6515cfa 100644
>> --- a/lib/trim.c
>> +++ b/lib/trim.c
>> @@ -65,7 +65,7 @@ trim2 (const char *s, int how)
>> /* Trim trailing whitespaces. */
>> if (how != TRIM_LEADING)
>> {
>> - int state = 0;
>> + unsigned int state = 0;
>> char *r IF_LINT (= NULL); /* used only while state = 2 */
>>
>> mbi_init (i, d, strlen (d));
>
> I don't care whether this variable is 'int' or 'unsigned int', but have you
> reported a GCC bug for this one? The variable 'state' is assigned only the
> values 0, 1, 2, always a constant right-hand side, and the only operation that
> is performed on it is ==. There is *nothing* dangerous about it.
>
> $ /arch/x86-linux/gnu-inst-gcc/4.6.0/bin/gcc -I . -I../.. -O2 -S
> trim.c -Wstrict-overflow
> trim.c: In function 'trim2':
> trim.c:81:18: warning: assuming signed overflow does not occur when
> simplifying conditional to constant [-Wstrict-overflow]
>
> The code in line 81 is as safe as the code in line 75 - for which no warning
> was issued.
>
> And whether the type is 'int' or 'unsigned int' should not matter at all
> because all possible values are 0, 1, 2.
Yes, I was surprised about that, too.
I'll probably report it tomorrow.
Looking at that code I noticed several useless assignments
and some formatting style glitches.
Fixed with this:
>From 44019e149cf40caa0b6d0eb57115eea555fdfee6 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@redhat.com>
Date: Sun, 29 May 2011 23:15:36 +0200
Subject: [PATCH] trim: remove three superfluous assignments
* lib/trim.c (trim2): Remove three superfluous assignments
and correct brace positioning.
---
ChangeLog | 6 ++++++
lib/trim.c | 33 +++++++++++++++------------------
2 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 9c8b3a9..8bcf087 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-05-29 Jim Meyering <meyer...@redhat.com>
+
+ trim: remove three superfluous assignments
+ * lib/trim.c (trim2): Remove three superfluous assignments
+ and correct brace positioning.
+
2011-05-29 Bruno Haible <br...@clisp.org>
wctype-h: Avoid namespace pollution on Solaris 2.6.
diff --git a/lib/trim.c b/lib/trim.c
index 6515cfa..155063e 100644
--- a/lib/trim.c
+++ b/lib/trim.c
@@ -73,10 +73,7 @@ trim2 (const char *s, int how)
for (; mbi_avail (i); mbi_advance (i))
{
if (state == 0 && mb_isspace (mbi_cur (i)))
- {
- state = 0;
- continue;
- }
+ continue;
if (state == 0 && !mb_isspace (mbi_cur (i)))
{
@@ -85,10 +82,7 @@ trim2 (const char *s, int how)
}
if (state == 1 && !mb_isspace (mbi_cur (i)))
- {
- state = 1;
- continue;
- }
+ continue;
if (state == 1 && mb_isspace (mbi_cur (i)))
{
@@ -97,7 +91,7 @@ trim2 (const char *s, int how)
}
else if (state == 2 && mb_isspace (mbi_cur (i)))
{
- state = 2;
+ /* empty */
}
else
{
@@ -114,18 +108,21 @@ trim2 (const char *s, int how)
char *p;
/* Trim leading whitespaces. */
- if (how != TRIM_TRAILING) {
- for (p = d; *p && isspace ((unsigned char) *p); p++)
- ;
+ if (how != TRIM_TRAILING)
+ {
+ for (p = d; *p && isspace ((unsigned char) *p); p++)
+ ;
- memmove (d, p, strlen (p) + 1);
- }
+ memmove (d, p, strlen (p) + 1);
+ }
/* Trim trailing whitespaces. */
- if (how != TRIM_LEADING) {
- for (p = d + strlen (d) - 1; p >= d && isspace ((unsigned char) *p);
p--)
- *p = '\0';
- }
+ if (how != TRIM_LEADING)
+ {
+ for (p = d + strlen (d) - 1;
+ p >= d && isspace ((unsigned char) *p); p--)
+ *p = '\0';
+ }
}
return d;
--
1.7.5.2.660.g9f46c
