[PATCH] D39538: [llvm-ar] Support an options string that start with a dash

[Martin Storsjö via llvm-commits]

On Fri, 3 Nov 2017, Rafael Avila de Espindola wrote:

> Martin Storsjö <martin at martin.st> writes:
>
>> On Thu, 2 Nov 2017, Rafael Avila de Espindola wrote:
>>
>>> Martin Storsjö via Phabricator <reviews at reviews.llvm.org> writes:
>>>
>>>> mstorsjo updated this revision to Diff 121361.
>>>> mstorsjo added a comment.
>>>>
>>>> Break out of the loop on '--' and once an argument with a stripped dash is found.
>>>>
>>>>
>>>> https://reviews.llvm.org/D39538
>>>>
>>>> Files:
>>>>   test/tools/llvm-ar/default-add.test
>>>>   tools/llvm-ar/llvm-ar.cpp
>>>>
>>>>
>>>> Index: tools/llvm-ar/llvm-ar.cpp
>>>> ===================================================================
>>>> --- tools/llvm-ar/llvm-ar.cpp
>>>> +++ tools/llvm-ar/llvm-ar.cpp
>>>> @@ -127,6 +127,8 @@
>>>>    "  [v] - be verbose about actions taken\n"
>>>>  );
>>>>
>>>> +static const char OptionChars[] = "dmpqrtxabiosSTucv";
>>>> +
>>>>  // This enumeration delineates the kinds of operations on an archive
>>>>  // that are permitted.
>>>>  enum ArchiveOperation {
>>>> @@ -879,6 +881,23 @@
>>>>        Stem.find("lib") != StringRef::npos)
>>>>      return libDriverMain(makeArrayRef(argv, argc));
>>>>
>>>> +  for (int i = 1; i < argc; i++) {
>>>> +    // If an argument starts with a dash and only contains chars
>>>> +    // that belong to the options chars set, remove the dash.
>>>> +    // We can't handle it after the command line options parsing
>>>> +    // is done, since it will error out on an unrecognized string
>>>> +    // starting with a dash.
>>>> +    // Make sure this doesn't match the actual llvm-ar specific options
>>>> +    // that start with a dash.
>>>> +    if (argv[i][0] == '-' &&
>>>> +        strspn(&argv[i][1], OptionChars) + 1 == strlen(argv[i])) {
>>>> +      argv[i]++;
>>>> +      break;
>>>> +    }
>>>> +    if (!strcmp(argv[i], "--"))
>>>> +      break;
>>>> +  }
>>>
>>> The intention is to not support "--format=gnu -rc"?
>>
>> No, that should work (since this is strcmp, not strncmp).
>>
>> The intention is to avoid issues with "llvm-ar rc foo.a -- -obj", where
>> "-obj" is an object file name that would otherwise be interpreted as a
>> potential ar command string, starting with a dash.
>
> Ah, could you use StringRef's ==? That should make it clear that it is
> not checking a prefix.

Ok, like StringRef(argv[i]) == "--"?

// Martin