confirming: compiles fine with gcc 6.4.0, fails on clang 5

That's what the G_GNUC_FORMAT on the function above it is for. If that doesn't work, that's clang's fault, not our code's. Try adding a prototype for it with the format and deleting the format from the implementation, or try moving the FORMAT before the static; does any of these fix this?

Well?

Sorry, had no time for that, currently set to it to be built with gcc

I don't think it's clang's fault.

G_GNUC_FORMAT applies to the parameter of format_as_comment() but not to its return value. I can't find a corresponding annotation for the latter. g_strdup_printf doesn't have anything like this either. So the compiler has no chance of knowing that the return value of this function is also a printf-format string.

This return value is then stored in commented_format, again, I can't find any way to denote that it's a format string; and later this is passed to g_vfprintf().

Hmmm, I might have been wrong here.

G_GNUC_FORMAT translates to __attribute__((__format_arg__(...))) for which https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes says:

    The format_arg attribute specifies that a function takes a format string for a printf, scanf, strftime or strfmon style function and modifies it [...]

    extern char *
    my_dgettext (char *my_domain, const char *my_format)
          __attribute__ ((format_arg (2)));

I wish it elaborated on that "modifies" word, but modifying in-place doesn't make any sense, so I guess it means to "return" the new value. In which case our code indeed looks correct to me.

> Try adding a prototype
> for it with the format and deleting the format from the implementation, or
> try moving the FORMAT before the static; does any of these fix this?

Nope.

It looks to me that there's nothing wrong with the G_GNUC_FORMAT(1) attribute. It could be moved to after the function name and parameters to be more consistent, but it's irrelevant.

What clang doesn't like is placing the result in char *commented_format. Looks to me that somehow it doesn't trust that just because we assign a certain value to this variable _here_, this variable will always have such a format value everywhere (in particular, at that g_vfprintf() below).

Clang is happy if we skip this intermediate variable:

  g_vfprintf(fp, format_as_comment (format), args);

or if we make it super-duper constant:

  const char * const commented_format = format_as_comment (format);

Note that both "const"s are required, omitting any of them results in the aforementioned error.

How to free the string in these cases, though?? gs_free is obviously rejected, one possibility is to free with an un-consting cast:

  g_free ((char *)commented_format);

which is quite ugly, but might be the best we can come up with.

Created attachment 364360 [details] [review]
Workaround

A note to my future self:

sudo apt install clang-5.0

CC=clang-5.0 CPP=clang-cpp-5.0 CXX=clang++-5.0 CXXCPP=clang-cpp-5.0 ./autogen.sh ...

How about

static const char * G_GNUC_FORMAT (2)
format_as_comment (char** freeme, const char *format)
{
  return *freeme = g_strdup_printf ("# %s", format);
}

and use as

gs_free char* freeme;
g_vfprintf(fp, format_as_comment(&freeme, format), args);

Compiles cleanly.

... or just simply print "# " and then vfprintf(fp, format, args).

It has a slim chance of some other tool that also produces output to intervene, although I guess this problem is negligible.

By the way, this whole thing with the "# " prefix, to be compatible with --print-environment as the comment in the file says, makes me wonder: Why do we print warning/error messages to stdout rather than stderr?

Created attachment 365631 [details] [review]
Workaround, v2

Let's not forget about this... Christian, please submit whichever workaround you prefer, thanks :)

Comment on attachment 365631 [details] [review]
Workaround, v2

This one please :-)

@Reporter: Please file a bug with clang for this erroneous warning.

(In reply to Christian Persch from comment #15)

> @Reporter: Please file a bug with clang for this erroneous warning.

Please point them to https://git.gnome.org/browse/gnome-terminal/tree/src/terminal-options.c?h=3.27.1#n44 lines 44-64 for the relevant code.

> This one please :-)

Submitted.

@Reporter: Please add a link to the clang bug on the See Also: field on this bug.