[m-rev.] diff: Fix compilation of memprof grades on 32-bit.

Julien Fischer jfischer at opturion.com
Fri Aug 10 14:35:00 AEST 2018


Hi Peter,

On Fri, 10 Aug 2018, Peter Wang wrote:

> On Fri, 10 Aug 2018 00:59:25 +0000 (UTC), Julien Fischer <jfischer at opturion.com> wrote:
>>
>> On Thu, 9 Aug 2018, Peter Wang wrote:
>>
>>> runtime/mercury_heap_profile.c:
>>>    Use MR_INTEGER_LENGTH_MODIFIER to get the right format specifier
>>>    when printing size_t fields.
>>>
>>> diff --git a/runtime/mercury_heap_profile.c b/runtime/mercury_heap_profile.c
>>> index d648f0113..f1e8ee1c3 100644
>>> --- a/runtime/mercury_heap_profile.c
>>> +++ b/runtime/mercury_heap_profile.c
>>> @@ -479,39 +479,43 @@ finish_reachable_report(const char *label)
>>>
>>> static void
>>> write_attrib_counts(FILE *fp, MR_AttribCount *table, size_t table_size)
>>> {
>>>     size_t i;
>>>
>>>     for (i = 0; i < table_size; i++) {
>>>         if (table[i].MR_atc_alloc_site != NULL &&
>>>             table[i].MR_atc_num_cells != 0)
>>>         {
>>> -            fprintf(fp, "%d %lu %lu\n",
>>> +            fprintf(fp, "%d "
>>> +                "%" MR_INTEGER_LENGTH_MODIFIER "u "
>>> +                "%" MR_INTEGER_LENGTH_MODIFIER "u\n",
>>
>> Since MR_atc_num_{cells,word} have ttype size_t, I think you
>> should use the "%zu" conversion specifier there.  (VS2017 supports
>> it, so there's no particular reason to avoid using it now.)
>
> Thanks, I didn't know of the 'z' specifier. But I don't think we should
> use it -- VS2015 is not very old and doesn't support it.

As far as VS is specifically concerned, I don't mind not supporting even
VS2015.  Maintaining support for older versions of VS has prevented us
from using many useful C99(!) features for many years.

> We might have to build on more obscure platforms with less well
> maintained C libraries too.

That's fair enough.

> There's no particular reason those fields must have type size_t
> (I wrote that code). MR_atc_num_cells, MR_atc_num_words, and
> MR_vsc_count are just counters. MR_vsc_size holds the size of allocated
> objects, but can just as well be 'MR_Unsigned' or even 'unsigned'
> really.
>
>>> static void
>>> write_var_size_counts(FILE *fp, const char *prefix, MR_VarSizeCount *node)
>>> {
>>>     while (node != NULL) {
>>>         write_var_size_counts(fp, prefix, node->MR_vsc_left);
>>>
>>>         if (node->MR_vsc_count != 0) {
>>> -            fprintf(fp, "%s %ld %ld\n",
>>> +            fprintf(fp, "%s "
>>> +                "%" MR_INTEGER_LENGTH_MODIFIER "d "
>>> +                "%" MR_INTEGER_LENGTH_MODIFIER "d\n",
>>
>> Ditto there; is there a reason we are printing those as
>> signed values there?  (Again, the fields have type size_t).
>
> No reason.

I suggest replacing them with 'u' then.

Julien


More information about the reviews mailing list