Todd Carson writes:
> Stuart Henderson writes:
>
>>> +make this function compile correctly under clang
>>> +Index: lib/utils/mu-str.c
>>> +--- lib/utils/mu-str.c.orig
>>> ++++ lib/utils/mu-str.c
>>> +@@ -49,7 +49,7 @@ mu_str_size_s (size_t s)
>>> + char*
>>> + mu_str_size (size_t s)
>>> + {
>>> +- return g_strdup (mu_str_size_s(s));
>>> ++ return g_format_size_for_display ((goffset)s);
>>> + }
>>
>> I'm no expert on glib2 but this seems a bit odd for "make this function
>> compile correctly under clang", have you talked to upstream about it
>> at all?
>>
>
> That patch is one I submitted, so I can elaborate.
>
> The two functions involved look like this (unpatched):
>
> const char*
> mu_str_size_s (size_t s)
> {
> static char buf[32];
> char *tmp;
>
> tmp = g_format_size_for_display ((goffset)s);
> strncpy (buf, tmp, sizeof(buf));
> buf[sizeof(buf) -1] = '\0'; /* just in case */
> g_free (tmp);
>
> return buf;
> }
>
> char*
> mu_str_size (size_t s)
> {
> return g_strdup (mu_str_size_s(s));
> }
>
> One of the tests was failing, and I found this was because the compiler
> was optimizing the function calls out of mu_str_size(), so that
> it directly returned the address of mu_str_size_s()'s static buffer.
>
> This happens with base-clang or ports-clang at -O1 or higher.
>
> I just tried ports-gcc and it compiles mu_str_size() correctly, but the
> build fails in the link stage. So maybe ports-gcc should be removed from
> COMPILER unless that can be fixed.
I looked at this some more, and the actual problem seems to be that
mu_str_size_s() is declared with the const attribute in mu-str.c.
I should have noticed that before; I wasn't looking at mu-str.h.
All the tests pass with base-clang at -O2 if that attribute is removed
from the declaration.
The mu-str.c patch can be replaced with this:
$OpenBSD$
Prevent the compiler from incorrectly optimizing mu_str_size()
Index: lib/utils/mu-str.h
--- lib/utils/mu-str.h.orig
+++ lib/utils/mu-str.h
@@ -48,7 +48,7 @@ G_BEGIN_DECLS
* @return a string representation of the size; see above
* for what to do with it
*/
-const char* mu_str_size_s (size_t s) G_GNUC_CONST;
+const char* mu_str_size_s (size_t s);
char* mu_str_size (size_t s) G_GNUC_WARN_UNUSED_RESULT;
I'll create a PR with upstream.
