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

Previous Next

Full log

Message #23 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, 16 Nov 2010 00:46:25 +0000

On 12/10/10 23:17, Pádraig Brady wrote:
> 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
> 
> 
> 
> 
I just applied that and this follow up

commit 0d1ba34494e5e5e21e205f2e86349afeb69ca84d
Author: Pádraig Brady <P <at> draigBrady.com>
Date:   Tue Nov 16 00:31:05 2010 +0000

    maint: fix a new -Wpointer-sign gcc warning

    * src/csplit.c (max_out): Fix a new warning introduced with
    commit 6568b173, 2010-11-10, "csplit: do not rely on..."

diff --git a/src/csplit.c b/src/csplit.c
index 531e492..07c5c8c 100644
--- a/src/csplit.c
+++ b/src/csplit.c
@@ -1275,7 +1275,7 @@ max_out (char *format)
           error (EXIT_FAILURE, 0,
                  _("too many %% conversion specifications in suffix"));
         percent = true;
-        unsigned int flags;
+        int flags;
         f += get_format_flags (f, &flags);
         while (ISDIGIT (*f))
           f++;

Previous Next

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