GNU bug report logs - #7203
[PATCH] sort: fix unportable conversion of unsigned char * -> char *

Previous Next

Full log

Message #14 received at 7203 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 7203 <at> debbugs.gnu.org, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#7203: [PATCH] sort: fix unportable conversion of unsigned
	char	* -> char *
Date: Tue, 12 Oct 2010 23:17:45 +0100

On 12/10/10 22:54, Pádraig Brady wrote:
> On 12/10/10 22:28, Jim Meyering wrote:
>> Paul Eggert wrote:
>>> This is another portability bug caught by the Sun C compiler.
>>> Without this patch, cc complains:
>>>
>>> "sort.c", line 3933: warning: assignment type mismatch:
>>>         pointer to const char "=" pointer to unsigned char
>>>
>>>
>>> * src/sort.c (fold_toupper): Change this back from char to
>>> unsigned char, fixing a regression introduced in commit
>>> 59e2e55d0f154a388adc9bac37d2b45f2ba971f8 dated February 26, as the
>>> C Standard doesn't let you convert from unsigned char * to char *
>>> without a cast, and the (in theory more portable) style here is to
>>> convert char values, not pointer values.
>>> (getmonth): Convert char to unsigned char when needed for
>>> comparison.
>>
>> Calling commit 59e2e55d0f a regression might lead the unwary
>> to think that it causes sort to malfunction, when in fact,
>> at worst (afaics), it evokes a warning.
>>
>> So how about something like s/regression/problem/ ?
>>
>> This feels like enough of a technicality that I'd prefer to defer it.
> 
> Note gcc would also warn about this with -Wall, except that
> we disable the warning with -Wno-pointer-sign in configure.ac
> 
> So I agree with the change, but s/regression/warning/

I just compiled coreutils/{src,lib} with make CFLAGS="-Wpointer-sign -Werror"
and the above was the only issue. How about enabling the warning?

commit e6d6fd6ecfce79b8c3f1e4ce7da67b26adbfbb82
Author: Pádraig Brady <P <at> draigBrady.com>
Date:   Tue Oct 12 23:15:23 2010 +0100

    maint: enable the -Wpointer-sign gcc warning

    * configure.ac: -Wall implicit enables this warning
    so remove the explicit disabling.

diff --git a/configure.ac b/configure.ac
index acd397e..7ebf98c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -98,7 +98,6 @@ if test "$gl_gcc_warnings" = yes; then
   done
   gl_WARN_ADD([-Wno-missing-field-initializers]) # We need this one
   gl_WARN_ADD([-Wno-sign-compare])     # Too many warnings for now
-  gl_WARN_ADD([-Wno-pointer-sign])     # Too many warnings for now
   gl_WARN_ADD([-Wno-unused-parameter]) # Too many warnings for now

   # In spite of excluding -Wlogical-op above, it is enabled, as of

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.