Re: [PING^3] Re: [PATCH 1/2] Add -B support to gcc-ar/ranlib/nm

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Aug 27, 2014 at 3:45 PM, Andi Kleen <andi@firstfloor.org> wrote:
> Andi Kleen <andi@firstfloor.org> writes:
>
> PING!
>
>> Andi Kleen <andi@firstfloor.org> writes:
>>
>> PING^2 !
>>
>> Would be nice to make slim bootstrap work, it really speeds it up quite
>> a bit.
>>
>>> From: Andi Kleen <ak@linux.intel.com>
>>>
>>> To use gcc-{ar,ranlib} for boot strap we need to add a -B option
>>> to the tool. Since ar has weird and unusual argument conventions
>>> implement the code by hand instead of using any libraries.
>>>
>>> v2: Fix typo
>>>
>>> gcc/:
>>>
>>> 2014-08-04  Andi Kleen  <ak@linux.intel.com>
>>>
>>>      * gcc-ar.c (main): Support -B option.
>>> ---
>>>  gcc/gcc-ar.c | 41 +++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 41 insertions(+)
>>>
>>> diff --git a/gcc/gcc-ar.c b/gcc/gcc-ar.c
>>> index aebaa92..70bf222 100644
>>> --- a/gcc/gcc-ar.c
>>> +++ b/gcc/gcc-ar.c
>>> @@ -132,9 +132,50 @@ main (int ac, char **av)
>>>    const char **nargv;
>>>    bool is_ar = !strcmp (PERSONALITY, "ar");
>>>    int exit_code = FATAL_EXIT_CODE;
>>> +  int i;
>>>
>>>    setup_prefixes (av[0]);
>>>
>>> +  /* Not using getopt for now.  */
>>> +  for (i = 0; i < ac; i++)
>>> +      if (!strncmp (av[i], "-B", 2))

This also matches joined -B/foo

>>> +    {
>>> +      const char *arg = av[i] + 2;
>>> +      const char *end;
>>> +
>>> +      memmove (av + i, av + i + 1, sizeof (char *) * ((ac + 1) - i));
>>> +      ac--;
>>> +      if (*arg == 0)
>>> +        {
>>> +          arg = av[i + 1];
>>> +          if (!arg)
>>> +            {

But this doesn't handle it?  common.opt has -B as Joined Separate option
thus allowing both.

>>> +              fprintf (stderr, "Usage: gcc-ar [-B prefix] ar arguments ...\n");
>>> +              exit (EXIT_FAILURE);
>>> +            }
>>> +          memmove (av + i, av + i + 1, sizeof (char *) * ((ac + 1) - i));
>>> +          ac--;
>>> +          i++;
>>> +        }
>>> +
>>> +      for (end = arg; *end; end++)
>>> +        ;
>>> +      end--;
>>> +      if (end > arg && *end != '/')
>>> +        {
>>> +          char *newarg = (char *)xmalloc (strlen(arg) + 2);
>>> +
>>> +          strcpy (newarg, arg);
>>> +          strcat (newarg, "/");
>>> +          arg = newarg;
>>> +        }

Why the above?  And why open-coded instead of using strlen?
Also instead of testing for '/' this should test for IS_DIR_SEPARATOR.

Without comments all this code is hard to decipher.

>>> +
>>> +      add_prefix (&path, arg);
>>> +      add_prefix (&target_path, arg);

This adds the -B path to the _end_ of the prefix list.  Does that match
gcc driver behavior?  The gcc driver uses PREFIX_PRIORITY_B_OPT
as argument to add_prefix which ends up adding -B prefixes to the
beginning of the prefix list.

Thanks,
Richard.

>>> +      break;
>>> +    }
>>> +
>>> +
>>>    /* Find the GCC LTO plugin */
>>>    plugin = find_a_file (&target_path, LTOPLUGINSONAME, R_OK);
>>>    if (!plugin)
>
> --
> ak@linux.intel.com -- Speaking for myself only