Fix bug in setting directories for tests.

tests/skia_test.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkCommandLineFlags.h"
 #include "SkGraphics.h"
 #include "SkOSFile.h"
@@ skipping 94 matching lines @@
     }
 
 private:
     int32_t fNextIndex;
     int32_t fPending;
     int fTotal;
     bool fAllowExtendedTest;
     bool fAllowThreaded;
 };
 
-static const char* make_canonical_dir_path(const char* path, SkString* storage) {
-    if (path) {
-        // clean it up so it always has a trailing searator
-        size_t len = strlen(path);
-        if (0 == len) {
-            path = NULL;
-        } else if (SkPATH_SEPARATOR != path[len - 1]) {
-            // resize to len + 1, to make room for searator
-            storage->set(path, len + 1);
-            storage->writable_str()[len] = SkPATH_SEPARATOR;
-            path = storage->c_str();
-        }
-    }
-    return path;
-}
-
 static SkString gTmpDir;

[epoger, 2013/06/04 15:15:34]

Hey, if this change fixes an existing problem, I'm in favor of committing it. 
But it seems to me that a cleaner approach would be:

1. remove global gTmpDir variable
2. rewrite Test::GetTmpDir() to just return an SkString derived from
FLAGS_tmpDir
3. adjust any callers to Test::GetTmpDir() to use SkOSPath::SkPathJoin() to
assemble their paths, rather than path.printf("%s%s", tmpDir, "data_test");

And, I suppose, the same sort of changes for gResourcePath.

[scroggo, 2013/06/04 19:50:39]

Done.
Done. I am not convinced this is better, but it seems just as good to me.
Done.
Interestingly, no one uses gResourcePath... When it was created it wasn't used,
but presumably a later change used it. I was about to delete it, but I plan on
using it with a future change.

[epoger, 2013/06/05 14:34:14]

My reasoning was: why use 2 global variables (gTmpDir and FLAGS_tmpDir) when
just 1 will do?

I guess the downside is that, because FLAGS_tmpDir is a char* instead of an
SkString, we need to create the SkString every time GetTmpDir() is called.  So
this is a performance hit.

I'm happy either way.

 
 const SkString& Test::GetTmpDir() {
     return gTmpDir;
 }
 
 static SkString gResourcePath;
 
 const SkString& Test::GetResourcePath() {
     return gResourcePath;
 }
@@ skipping 70 matching lines @@
     }
     return !anyExclude;
 }
 
 int tool_main(int argc, char** argv);
 int tool_main(int argc, char** argv) {
     SkCommandLineFlags::SetUsage("");
     SkCommandLineFlags::Parse(argc, argv);
 
     if (!FLAGS_tmpDir.isEmpty()) {
-        make_canonical_dir_path(FLAGS_tmpDir[0], &gTmpDir);
+        gTmpDir = SkOSPath::SkPathJoin(FLAGS_tmpDir[0], NULL);
     }
     if (!FLAGS_resourcePath.isEmpty()) {
-        make_canonical_dir_path(FLAGS_resourcePath[0], &gResourcePath);
+        gResourcePath = SkOSPath::SkPathJoin(FLAGS_resourcePath[0], NULL);
     }
 
 #if SK_ENABLE_INST_COUNT
     gPrintInstCount = true;
 #endif
 
     SkGraphics::Init();
 
     {
         SkString header("Skia UnitTests:");
@@ skipping 84 matching lines @@
     GpuTest::DestroyContexts();
 
     return (failCount == 0) ? 0 : 1;
 }
 
 #if !defined(SK_BUILD_FOR_IOS) && !defined(SK_BUILD_FOR_NACL)
 int main(int argc, char * const argv[]) {
     return tool_main(argc, (char**) argv);
 }
 #endif