This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, c++ diagnostics] Fix c++ front end i18n problems


Hi,
Thanks comment.
On 12/10/2009 07:34 PM, Paolo Bonzini wrote:
You still have memory leaks. Also, the condition "fns == NULL_TREE && !TREE_VALUE (fns)" is wrong as the second part of the "&&" will always cause a segfault when evaluated.

+  /* Only if all the candidates are covered,
+     the spaces can be freed.  */
+  if (spaces
+      && fns == NULL_TREE
+      && !TREE_VALUE (fns))
+    {
+       *str = NULL;
+       free (spaces);
+    }

if (!spaces) { /* String owned by the owner, it should not be freed. */ *str = NULL; }

In order to print " "<repeat n times> from the second candidate at


error ("%s %+#D", *str, OVL_CURRENT (fn));

The *str is changed at print_overloaded_functions first. When the OVL_NEXT loop is over, and back to print_chained_functions, call print_overloaded_functions will transfer " "<repeat n times> to print_overloaded_functions. That is also why the argument of print_overloaded_functions is **str not *str.
Since the pointer spaces and *str point the same place after assign spaces to *str, *str can't be assign NULL. Otherwise it will produce "segment fault".
  else if (!fns || !TREE_CHAIN (fns))
    {
      *str = NULL;
      free (spaces);
    }

You also need a big big comment above print_overloded_functions saying how STR should be used and freed.

+static void
+print_chained_functions (tree fns, const char *str)
+{
+  tree fn;
+  for (fn = fns; fn != NULL_TREE; fn = TREE_CHAIN (fn))
+    print_overloaded_functions (TREE_VALUE (fn),&str);

if (str) free (str);

(I know that free (NULL) is portable, however in this case I think it is better to have "if (str)" because it matches the interface of print_overloaded_functions).

+       if (!OVL_NEXT(fns))
+         str = _("candidate is:");
+       print_overloaded_functions (fns,&str);

Coding standards (space after comma).


if (str)
  free (str);

The code is a bit unwieldy however. I suggest you do the following:

- rename what's currently print_overloaded_functions to print_candidate_functions

- move the choice of _("candidate is:") vs. _("candidate are:") to print_chained_functions as well as to a new wrapper function


static void print_overloaded_functions (tree fns)


The const char *str argument can be removed from print_chained_functions as well.

I hope this is clear, thanks!

Paolo


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]