[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#7203: [PATCH] sort: fix unportable conversion of unsigned char * ->
From: |
Pádraig Brady |
Subject: |
bug#7203: [PATCH] sort: fix unportable conversion of unsigned char * -> char * |
Date: |
Tue, 16 Nov 2010 00:46:25 +0000 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 |
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 <address@hidden>
> 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 <address@hidden>
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++;
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- bug#7203: [PATCH] sort: fix unportable conversion of unsigned char * -> char *,
Pádraig Brady <=