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

Peter Wang novalazy at gmail.com
Fri Aug 10 14:25:40 AEST 2018


Hi Julien,

On Fri, 10 Aug 2018 00:59:25 +0000 (UTC), Julien Fischer <jfischer at opturion.com> wrote:
> 
> Hi Peter,
> 
> 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. We might have
to build on more obscure platforms with less well maintained C libraries
too.

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.

Peter


More information about the reviews mailing list