On Wed, May 22, 2019 at 10:07:33AM +0200, Martin Liška wrote:
> On 5/22/19 9:50 AM, Jakub Jelinek wrote:
> > On Wed, May 22, 2019 at 09:39:52AM +0200, Martin Liška wrote:
> >> The patch is about using of uintptr_t instead unsigned long that's
> >> being used for printing value of a pointer.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> >
> > No. You haven't adjusted the fprintf format strings for this change,
> > so it will break build on any target where uintptr_t is not unsigned long.
> >
> > I don't see any gcc/ code using the PRIxPTR and similar macros, so no idea
> > about the portability of that, but I'd think we just should keep that as is.
>
> Ok.
If the problem is just warnings, perhaps you could do
(unsigned long) (uintptr_t) casts instead of directly to (unsigned long),
ideally only for the options with pointer types.
Seems we have:
else if (otype ~ "^const char \\**$")
var_opt_string[n_opt_string++] = name;
else
var_opt_other[n_opt_other++] = name;
and so the
print " fputs (\"\\n\", file);";
for (i = 0; i < n_opt_other; i++) {
print " if (ptr1->x_" var_opt_other[i] " != ptr2->x_" var_opt_other[i]
")";
print " fprintf (file, \"%*s%s (%#lx/%#lx)\\n\",";
print " indent_to, \"\",";
print " \"" var_opt_other[i] "\",";
print " (unsigned long)ptr1->x_" var_opt_other[i] ",";
print " (unsigned long)ptr2->x_" var_opt_other[i] ");";
print "";
}
printing isn't done for strings, we use in that case
for (i = 0; i < n_opt_string; i++) {
name = var_opt_string[i]
print " if (ptr1->x_" name " != ptr2->x_" name "";
print " || (!ptr1->x_" name" || !ptr2->x_" name
print " || strcmp (ptr1->x_" name", ptr2->x_" name ")))";
print " fprintf (file, \"%*s%s (%s/%s)\\n\",";
print " indent_to, \"\",";
print " \"" name "\",";
print " ptr1->x_" name ",";
print " ptr2->x_" name ");";
print "";
}
instead. So, the only problem is in:
print " fputs (\"\\n\", file);";
for (i = 0; i < n_target_other; i++) {
print " if (ptr->x_" var_target_other[i] ")";
hwi = host_wide_int[var_target_other[i]]
if (hwi == "yes")
print " fprintf (file, \"%*s%s (%#\" HOST_WIDE_INT_PRINT
\"x)\\n\",";
else
print " fprintf (file, \"%*s%s (%#lx)\\n\",";
print " indent, \"\",";
print " \"" var_target_other[i] "\",";
if (hwi == "yes")
print " ptr->x_" var_target_other[i] ");";
else
print " (unsigned long)ptr->x_" var_target_other[i]
");";
print "";
}
and another similar block with ptr1/ptr2 later. So, I wonder if you just
shouldn't follow what is done for var_opt_string vs. var_opt_other even in
the var_target_other case, do:
+ else if (otype ~ "^const char \\**$")
+ var_target_string[n_target_string++] = name;
else
var_target_other[n_target_other++] = name;
+ initialization etc. and handle var_target_string differently from
var_target_other for the printing (print it actually with %s instead of
%#lx).
Jakub
