Re: Ping - old patch from April - mingw support for I32/I64 MS printf formatters to c-format.c

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Wed, 16 Jan 2008, Kai Tietz wrote:

> ChangeLog of gcc/:
> 2008-01-15      Kai Tietz  <kai.tietz@onevision.com>
> 
>         * gcc/c-format.h: Add structure target_ovr_attr to hold

You need to remove the gcc/ from this and subsequent names in this 
ChangeLog.

> +static const char *
> +replace_formatter_name_to_system_name (const char *attr_name)

You need a comment above this function explaining what it does (including 
the significance of the parameter and the return value.

> +  if (attr_name == NULL || *attr_name == 0 || strncmp (attr_name, "gcc_", 4) == 0)

Please try to keep source lines under 80 characters wide, here and 
elsewhere.

> +/* Compare the target override format attribute by treating double underscore syntax.  */
> +static int
> +compare_tofa (const char *tattr_name, const char *attr_name)

The comment needs to explain the parameters and the return value.  "tofa" 
is also cryptic; a longer, more descriptive, function name might be 
better.

> +/* For custom types we need to compare in a special way.  */
> +static int compare_format_types (int custom_type, int def_type)

Again, explain the parameters and return types; this comment isn't 
explaining the function interface at all.  The formatting should be:

static int
compare_format_types (int custom_type, int def_type)

> +@defmac TARGET_OVERRIDES_FORMAT_ATTRIBUTES
> +If defined, this macro is the name of a global variable containg
> +target-specific format overrides for the @option{-Wformat} option. The
> +default is to have no target-specific format overrides. This depends, that
> +@code{TARGET_FORMAT_TYPES} is defined too.
> +@end defmac

The last sentence "This depends, that code{TARGET_FORMAT_TYPES} is defined 
too." doesn't make sense to me.

> +@defmac TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT
> +If defined, this macro is the number of entries in
> +@code{}.
> +@end defmac

This (in particular the empty @code{}) is clearly incomplete.

> Index: gcc/gcc/testsuite/gcc.dg/format/c90-printf-3.c
> ===================================================================
> --- gcc.orig/gcc/testsuite/gcc.dg/format/c90-printf-3.c
> +++ gcc/gcc/testsuite/gcc.dg/format/c90-printf-3.c
> @@ -17,8 +17,8 @@ foo (int i, char *s, size_t n, va_list v
>    printf ("%d", i);
>    printf ("%ld", i); /* { dg-warning "format" "printf" } */
>    /* The "unlocked" functions shouldn't warn in c90 mode.  */
> -  fprintf_unlocked (stdout, "%ld", i); /* { dg-bogus "format" "fprintf_unlocked" } */
> -  printf_unlocked ("%ld", i); /* { dg-bogus "format" "printf_unlocked" } */
> +  fprintf_unlocked (stdout, "%ld", i); /* { dg-warning "format" "fprintf_unlocked" } */
> +  printf_unlocked ("%ld", i); /* { dg-warning "format" "printf_unlocked" } */

You are changing the test in a way that contradicts its purpose.  The 
whole point of the tests of non-C90 functions in this file is to assert 
that those functions do not receive default attributes in strict C90 mode; 
you'll need to find some way to avoid giving them explicit attributes for 
this file.

> Index: gcc/gcc/testsuite/gcc.dg/format/c90-scanf-4.c

Likewise here.

> Index: gcc/gcc/testsuite/gcc.dg/format/c99-printf-3.c

Likewise here.

-- 
Joseph S. Myers
joseph@codesourcery.com