https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63853

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Dominique d'Humieres from comment #9)
> I just completed a clean bootstrap of r217511 with the following patch

This is ok for trunk so that we don't have bootstrap breakages for too long.

That said, most of the strchrnul uses look to me like completely unnecessary.

> --- ../_clean/gcc/gcc.c       2014-11-13 15:29:00.000000000 +0100
> +++ gcc/gcc.c 2014-11-13 17:59:47.000000000 +0100
> @@ -3375,12 +3375,16 @@ handle_foffload_option (const char *arg)
>    if (arg[0] == '-')
>      return;
>  
> -  end = strchrnul (arg, '=');
> +  end = strchr (arg, '=');
> +  if (end == NULL)
> +    end = strchr (arg, '\0');
>    cur = arg;
>  
>    while (cur < end)
>      {
> -      next = strchrnul (cur, ',');
> +      next = strchr (cur, ',');
> +      if (next == NULL)
> +     next = strchr (cur, '\0');

E.g. here.  If strchr (cur, ',') returns NULL, then what
strchr (cur, '\0'); returns is either equal to end, or > end, so we can just
set next = end;.

>        strncpy (target, cur, next - cur);

memcpy should be used instead.

> -       n = strchrnul (c, ',');
> +       n = strchr (c, ',');
> +       if (n == NULL)
> +         n = strchr (c, '\0');
>  
>         if (strlen (target) == (size_t) (n - c)

Here, strlen (target) should be known, isn't it next - cur ?
(and another spot with that).

Also,
  offload_targets = xstrdup (target);
(could have instead just set offload_targets = target;
and ensure it isn't freed, say by setting target = NULL).

And:
             offload_targets
                = XRESIZEVEC (char, offload_targets,
                              strlen (offload_targets) + strlen (target) + 2);
              if (strlen (offload_targets) != 0)
                strcat (offload_targets, ":");
              strcat (offload_targets, target);
Compute strlen (offload_targets) once, strlen (target) is known (next - cur),
and don't compute it 5 times instead.
              size_t offload_targets_len = strlen (offload_targets);
              offload_targets = XRESIZEVEC (char, offload_targets,
                                            offload_targets_len + (next - cur)
+ 2);
              if (offload_targets_len)
                offload_targets[offload_targets_len++] = ':';
              memcpy (offload_targets + offload_targets_len, target, next -
cur);
?  Especially if (strlen (offload_targets) != 0) is very expensive way of
if (*offload_targets)
The tree-ssa-strlen.c pass will presumably optimize some of this, but given
the conditional likely not all of that.

Sorry for not catching that during review.
s + strcspn (s, "=") instead of strchrnul (s, '=') is reasonable too.

Reply via email to