On Wed, 18 May 2011 m...@freebsd.org wrote:
On Wed, May 18, 2011 at 2:31 PM, Dimitry Andric <d...@freebsd.org> wrote:
On 2011-05-18 23:16, Pawel Jakub Dawidek wrote:
On Wed, May 18, 2011 at 09:06:20PM +0000, Ben Laurie wrote:
Author: benl
Date: Wed May 18 21:06:20 2011
New Revision: 222084
URL: http://svn.freebsd.org/changeset/base/222084
Log:
? Fix clang warnings.
? Approved by: philip (mentor)
[...]
- ? ? ? ? ? ?fprintf (stderr, " by changing asso_value['%c'] (char #%d)
to %d\n",
+ ? ? ? ? ? ?fprintf (stderr, " by changing asso_value['%c'] (char #%zd)
to %d\n",
? ? ? ? ? ? ? ? ? ? ? *p, p - union_set + 1, asso_values[(unsigned
char)(*p)]);
Hmm, both 'p' and 'union_set' are 'char *' and %zd is for ssize_t. It is
a bit strange that it fixes the warning.
If you subtract two pointers, such as in this case, you get a ptrdiff_t.
Strictly, this doesn't have to be exactly the same type as ssize_t, but
in practice it will almost always be.
You can also cast the result to intmax_t, and use %jd, then it will
always be correct, but possibly have some small overhead.
Or you can use %td which is the C99 conversion specifier for ptrdiff_t.
Of course this is the only correct fix.
All the changes are wrong IMO. Apart from being unmaintainable since they
are in dusty contrib code:
% Modified: head/contrib/gperf/src/gen-perf.cc
% ==============================================================================
% --- head/contrib/gperf/src/gen-perf.cc Wed May 18 21:04:29 2011
(r222083)
% +++ head/contrib/gperf/src/gen-perf.cc Wed May 18 21:06:20 2011
(r222084)
% @@ -246,7 +246,7 @@ Gen_Perf::change (List_Node *prior, List
% {
% if (option[DEBUG])
% {
% - fprintf (stderr, " by changing asso_value['%c'] (char #%d) to
%d\n",
% + fprintf (stderr, " by changing asso_value['%c'] (char #%zd) to
%d\n",
% *p, p - union_set + 1, asso_values[(unsigned
char)(*p)]);
% fflush (stderr);
% }
%
%td
% Modified: head/contrib/gperf/src/key-list.cc
% ==============================================================================
% --- head/contrib/gperf/src/key-list.cc Wed May 18 21:04:29 2011
(r222083)
% +++ head/contrib/gperf/src/key-list.cc Wed May 18 21:06:20 2011
(r222084)
% @@ -497,8 +497,8 @@ Key_List::merge (List_Node *list1, List_
% *resultp = list1;
% break;
% }
% - if (occurrence_sort && list1->occurrence < list2->occurrence
% - || hash_sort && list1->hash_value > list2->hash_value)
% + if ((occurrence_sort && list1->occurrence < list2->occurrence)
% + || (hash_sort && list1->hash_value > list2->hash_value))
% {
% *resultp = list2;
% resultp = &list2->next; list2 = list1; list1 = *resultp;
It is a compiler bug to warn about precedence when there is no problem
with precedence, as here for && vs ||. clang recently became even
more broken than gcc for this -- it now warns even without -Wparentheses
(or -Wall, which implies -Wparentheses) in CFLAGS, so it issues broken
warning at very low WARNS levels (for WARNS=1, maybe even with no
WARNS).
% @@ -1035,17 +1035,16 @@ Key_List::output_hash_function (void)
% if (option[CPLUSPLUS])
% printf ("%s::", option.get_class_name ());
% printf ("%s ", option.get_hash_name ());
% - printf (option[KRC] ?
% - "(str, len)\n"
% - " register char *str;\n"
% - " register unsigned int len;\n" :
% - option[C] ?
% - "(str, len)\n"
% - " register const char *str;\n"
% - " register unsigned int len;\n" :
% - option[ANSIC] | option[CPLUSPLUS] ?
% - "(register const char *str, register unsigned int len)\n" :
% - "");
% + if (option[KRC] || option[C] || option [ANSIC] || option[CPLUSPLUS])
% + printf (option[KRC] ?
% + "(str, len)\n"
% + " register char *str;\n"
% + " register unsigned int len;\n" :
% + option[C] ?
% + "(str, len)\n"
% + " register const char *str;\n"
% + " register unsigned int len;\n" :
% + "(register const char *str, register unsigned int len)\n");
%
% /* Note that when the hash function is called, it has already been verified
% that min_key_len <= len <= max_key_len. */
Far too invasive, and I can't even see a problem in the original. The
original has an empty format for the default case. This is perfectly valid,
an serves as documentation for the default case. The expression is a
"computed switch" statement. The change also obfuscates the pseudo-cases
option[ANSIC] and option[CPLUSPLUS] as the default pseudo-case (after
filtering out the actual default case before the pseudo-switch.
% @@ -1442,7 +1441,7 @@ Key_List::output_lookup_array (void)
%
% if (option[DEBUG])
% fprintf (stderr,
% - "dup_ptr[%d]: hash_value = %d, index = %d, count =
%d\n",
% + "dup_ptr[%zd]: hash_value = %d, index = %d, count =
%d\n",
% dup_ptr - duplicates,
% dup_ptr->hash_value, dup_ptr->index, dup_ptr->count);
%
Looks like another ptrdiff_t.
% @@ -1986,17 +1985,16 @@ Key_List::output_lookup_function (void)
% if (option[CPLUSPLUS])
% printf ("%s::", option.get_class_name ());
% printf ("%s ", option.get_function_name ());
% - printf (option[KRC] ?
% - "(str, len)\n"
% - " register char *str;\n"
% - " register unsigned int len;\n" :
% - option[C] ?
% - "(str, len)\n"
% - " register const char *str;\n"
% - " register unsigned int len;\n" :
% - option[ANSIC] | option[CPLUSPLUS] ?
% - "(register const char *str, register unsigned int len)\n" :
% - "");
% + if (option[KRC] || option[C] || option[ANSIC] || option[CPLUSPLUS])
% + printf (option[KRC] ?
% + "(str, len)\n"
% + " register char *str;\n"
% + " register unsigned int len;\n" :
% + option[C] ?
% + "(str, len)\n"
% + " register const char *str;\n"
% + " register unsigned int len;\n" :
% + "(register const char *str, register unsigned int len)\n");
%
% /* Output the function's body. */
% printf ("{\n");
%
Another pseudo-switch invaded.
% Modified: head/contrib/gperf/src/options.cc
% ==============================================================================
% --- head/contrib/gperf/src/options.cc Wed May 18 21:04:29 2011
(r222083)
% +++ head/contrib/gperf/src/options.cc Wed May 18 21:06:20 2011
(r222084)
% @@ -237,7 +237,7 @@ Options::print_options (void)
% {
% putchar (*arg);
% arg++;
% - if (*arg >= 'A' && *arg <= 'Z' || *arg >= 'a' && *arg <= 'z')
% + if ((*arg >= 'A' && *arg <= 'Z') || (*arg >= 'a' && *arg <= 'z'))
% {
% putchar (*arg);
% arg++;
%
Another precedence non-problem.
Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"