[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