[llvm-commits] [llvm] r82675 - in /llvm/trunk: lib/Support/CommandLine.cpp unittests/Support/CommandLineTest.cpp

[Jeffrey Yasskin]

The patch I sent should skip the test if you have neither setenv or
_putenv_s, but since _putenv_s doesn't help you run the test, I'll
remove it and send another patch.

On Fri, Sep 25, 2009 at 11:34 AM, Sandeep Patel <deeppatel1987 at gmail.com> wrote:
> This won't work for me. I build our Windows-hosted tools using MinGW.
> I'm not building using native Windows headers. mingwrt-3.15.2 lacks
> _putenv_s in it's headers.
>
> It may just be easier to skip this test on Windows.
>
> deep
>
> On Fri, Sep 25, 2009 at 5:28 PM, Jeffrey Yasskin <jyasskin at google.com> wrote:
>> Could you check if the attached patch fixes things for you? And if it
>> looks sensible? It includes the new generated configure script so you
>> don't have to regenerate it.
>>
>> On Fri, Sep 25, 2009 at 8:39 AM, Jeffrey Yasskin <jyasskin at google.com> wrote:
>>> Oops. :( Patch coming up. It appears MinGW intends us to use putenv,
>>> but I may just skip the test when setenv isn't available.
>>>
>>> On Fri, Sep 25, 2009 at 12:46 AM, Sandeep Patel <deeppatel1987 at gmail.com> wrote:
>>>> CommandLineTest.cpp breaks MinGW, which lacks setenv/unsetenv.
>>>>
>>>> deep
>>>>
>>>> On Thu, Sep 24, 2009 at 1:14 AM, Jeffrey Yasskin <jyasskin at google.com> wrote:
>>>>> Author: jyasskin
>>>>> Date: Wed Sep 23 20:14:07 2009
>>>>> New Revision: 82675
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=82675&view=rev
>>>>> Log:
>>>>> Roll back r82348, which introduced an infinite loop in ParseCStringVector() that
>>>>> a trivial unittest would have caught.  This revision also adds the trivial
>>>>> unittest.
>>>>>
>>>>>
>>>>> Added:
>>>>>    llvm/trunk/unittests/Support/CommandLineTest.cpp
>>>>> Modified:
>>>>>    llvm/trunk/lib/Support/CommandLine.cpp
>>>>>
>>>>> Modified: llvm/trunk/lib/Support/CommandLine.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=82675&r1=82674&r2=82675&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/Support/CommandLine.cpp (original)
>>>>> +++ llvm/trunk/lib/Support/CommandLine.cpp Wed Sep 23 20:14:07 2009
>>>>> @@ -351,31 +351,42 @@
>>>>>  /// using strdup(), so it is the caller's responsibility to free()
>>>>>  /// them later.
>>>>>  ///
>>>>> -static void ParseCStringVector(std::vector<char *> &OutputVector,
>>>>> -                               const char *Input) {
>>>>> +static void ParseCStringVector(std::vector<char *> &output,
>>>>> +                               const char *input) {
>>>>>   // Characters which will be treated as token separators:
>>>>> -  StringRef Delims = " \v\f\t\r\n";
>>>>> +  static const char *const delims = " \v\f\t\r\n";
>>>>>
>>>>> -  StringRef WorkStr(Input);
>>>>> -  while (!WorkStr.empty()) {
>>>>> -    // If the first character is a delimiter, strip them off.
>>>>> -    if (Delims.find(WorkStr[0]) != StringRef::npos) {
>>>>> -      size_t Pos = WorkStr.find_first_not_of(Delims);
>>>>> -      if (Pos == StringRef::npos) Pos = WorkStr.size();
>>>>> -      WorkStr = WorkStr.substr(Pos);
>>>>> -      continue;
>>>>> +  std::string work(input);
>>>>> +  // Skip past any delims at head of input string.
>>>>> +  size_t pos = work.find_first_not_of(delims);
>>>>> +  // If the string consists entirely of delims, then exit early.
>>>>> +  if (pos == std::string::npos) return;
>>>>> +  // Otherwise, jump forward to beginning of first word.
>>>>> +  work = work.substr(pos);
>>>>> +  // Find position of first delimiter.
>>>>> +  pos = work.find_first_of(delims);
>>>>> +
>>>>> +  while (!work.empty() && pos != std::string::npos) {
>>>>> +    // Everything from 0 to POS is the next word to copy.
>>>>> +    output.push_back(strdup(work.substr(0,pos).c_str()));
>>>>> +    // Is there another word in the string?
>>>>> +    size_t nextpos = work.find_first_not_of(delims, pos + 1);
>>>>> +    if (nextpos != std::string::npos) {
>>>>> +      // Yes? Then remove delims from beginning ...
>>>>> +      work = work.substr(work.find_first_not_of(delims, pos + 1));
>>>>> +      // and find the end of the word.
>>>>> +      pos = work.find_first_of(delims);
>>>>> +    } else {
>>>>> +      // No? (Remainder of string is delims.) End the loop.
>>>>> +      work = "";
>>>>> +      pos = std::string::npos;
>>>>>     }
>>>>> -
>>>>> -    // Find position of first delimiter.
>>>>> -    size_t Pos = WorkStr.find_first_of(Delims);
>>>>> -    if (Pos == StringRef::npos) Pos = WorkStr.size();
>>>>> -
>>>>> -    // Everything from 0 to Pos is the next word to copy.
>>>>> -    char *NewStr = (char*)malloc(Pos+1);
>>>>> -    memcpy(NewStr, WorkStr.data(), Pos);
>>>>> -    NewStr[Pos] = 0;
>>>>> -    OutputVector.push_back(NewStr);
>>>>>   }
>>>>> +
>>>>> +  // If `input' ended with non-delim char, then we'll get here with
>>>>> +  // the last word of `input' in `work'; copy it now.
>>>>> +  if (!work.empty())
>>>>> +    output.push_back(strdup(work.c_str()));
>>>>>  }
>>>>>
>>>>>  /// ParseEnvironmentOptions - An alternative entry point to the
>>>>>
>>>>> Added: llvm/trunk/unittests/Support/CommandLineTest.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CommandLineTest.cpp?rev=82675&view=auto
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/unittests/Support/CommandLineTest.cpp (added)
>>>>> +++ llvm/trunk/unittests/Support/CommandLineTest.cpp Wed Sep 23 20:14:07 2009
>>>>> @@ -0,0 +1,48 @@
>>>>> +//===- llvm/unittest/Support/CommandLineTest.cpp - CommandLine tests ------===//
>>>>> +//
>>>>> +//                     The LLVM Compiler Infrastructure
>>>>> +//
>>>>> +// This file is distributed under the University of Illinois Open Source
>>>>> +// License. See LICENSE.TXT for details.
>>>>> +//
>>>>> +//===----------------------------------------------------------------------===//
>>>>> +
>>>>> +#include "llvm/Support/CommandLine.h"
>>>>> +
>>>>> +#include "gtest/gtest.h"
>>>>> +
>>>>> +#include <string>
>>>>> +#include <stdlib.h>
>>>>> +
>>>>> +using namespace llvm;
>>>>> +
>>>>> +namespace {
>>>>> +
>>>>> +class TempEnvVar {
>>>>> + public:
>>>>> +  TempEnvVar(const char *name, const char *value)
>>>>> +      : name(name) {
>>>>> +    const char *old_value = getenv(name);
>>>>> +    EXPECT_EQ(NULL, old_value) << old_value;
>>>>> +    setenv(name, value, true);
>>>>> +  }
>>>>> +
>>>>> +  ~TempEnvVar() {
>>>>> +    unsetenv(name);
>>>>> +  }
>>>>> +
>>>>> + private:
>>>>> +  const char *const name;
>>>>> +};
>>>>> +
>>>>> +const char test_env_var[] = "LLVM_TEST_COMMAND_LINE_FLAGS";
>>>>> +
>>>>> +cl::opt<std::string> EnvironmentTestOption("env-test-opt");
>>>>> +TEST(CommandLineTest, ParseEnvironment) {
>>>>> +  TempEnvVar TEV(test_env_var, "-env-test-opt=hello");
>>>>> +  EXPECT_EQ("", EnvironmentTestOption);
>>>>> +  cl::ParseEnvironmentOptions("CommandLineTest", test_env_var);
>>>>> +  EXPECT_EQ("hello", EnvironmentTestOption);
>>>>> +}
>>>>> +
>>>>> +}  // anonymous namespace
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>>
>>
>