[PATCH] D40606: [Support/TarWriter] - Don't allow TarWriter to add the same file more than once.

[Rafael Avila de Espindola via llvm-commits]

The code change LGTM.

The TarWriter is tested in llvm with unittests. Can you add a test
showing that if the same path is added twice we only keep the first one?

Thanks,
Rafael

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 124958.
> grimar added a comment.
>
> - Use llvm::StringSet instead of std::set<std::string>.
>
>
> https://reviews.llvm.org/D40606
>
> Files:
>   include/llvm/Support/TarWriter.h
>   lib/Support/TarWriter.cpp
>
>
> Index: lib/Support/TarWriter.cpp
> ===================================================================
> --- lib/Support/TarWriter.cpp
> +++ lib/Support/TarWriter.cpp
> @@ -173,6 +173,10 @@
>    // Write Path and Data.
>    std::string Fullpath = BaseDir + "/" + sys::path::convert_to_slash(Path);
>  
> +  // We do not want to include the same file more than once.
> +  if (!Files.insert(Fullpath).second)
> +    return;
> +
>    StringRef Prefix;
>    StringRef Name;
>    if (splitUstar(Fullpath, Prefix, Name)) {
> Index: include/llvm/Support/TarWriter.h
> ===================================================================
> --- include/llvm/Support/TarWriter.h
> +++ include/llvm/Support/TarWriter.h
> @@ -11,6 +11,7 @@
>  #define LLVM_SUPPORT_TAR_WRITER_H
>  
>  #include "llvm/ADT/StringRef.h"
> +#include "llvm/ADT/StringSet.h"
>  #include "llvm/Support/Error.h"
>  #include "llvm/Support/raw_ostream.h"
>  
> @@ -26,6 +27,7 @@
>    TarWriter(int FD, StringRef BaseDir);
>    raw_fd_ostream OS;
>    std::string BaseDir;
> +  StringSet<> Files;
>  };
>  }
>  
>
>
> Index: lib/Support/TarWriter.cpp
> ===================================================================
> --- lib/Support/TarWriter.cpp
> +++ lib/Support/TarWriter.cpp
> @@ -173,6 +173,10 @@
>    // Write Path and Data.
>    std::string Fullpath = BaseDir + "/" + sys::path::convert_to_slash(Path);
>  
> +  // We do not want to include the same file more than once.
> +  if (!Files.insert(Fullpath).second)
> +    return;
> +
>    StringRef Prefix;
>    StringRef Name;
>    if (splitUstar(Fullpath, Prefix, Name)) {
> Index: include/llvm/Support/TarWriter.h
> ===================================================================
> --- include/llvm/Support/TarWriter.h
> +++ include/llvm/Support/TarWriter.h
> @@ -11,6 +11,7 @@
>  #define LLVM_SUPPORT_TAR_WRITER_H
>  
>  #include "llvm/ADT/StringRef.h"
> +#include "llvm/ADT/StringSet.h"
>  #include "llvm/Support/Error.h"
>  #include "llvm/Support/raw_ostream.h"
>  
> @@ -26,6 +27,7 @@
>    TarWriter(int FD, StringRef BaseDir);
>    raw_fd_ostream OS;
>    std::string BaseDir;
> +  StringSet<> Files;
>  };
>  }
>