[PATCH] D28091: Add a class to create a tar archive file.

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 5 14:22:51 PST 2017


Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:
> +static std::string formatPax(StringRef Key, const Twine &Val) {
> +  int Len = Key.size() + Val.str().size() + 3; // +3 for " ", "=" and
> "\n"

Nit: Save the temporary str() and you can reused them in the function.

> +// Headers in tar files must be aligned to 512 byte boundaries.
> +// This function writes null bytes so that the file is a multiple
> +// of 512 bytes.
> +static void pad(raw_ostream &OS) {
> +  uint64_t Pos = OS.tell();
> +  if (Pos % BlockSize == 0)
> +    return;
> +  OS << std::string(BlockSize - Pos % BlockSize, '\0');

Nit: Don't allocate a string for this.

> +// We want to use '/' as a path separator even on Windows.
> +// This function canonicalizes a given path.
> +static std::string canonicalize(StringRef S) {

this is lld::convertToUnixPathSeparator.


This is a lot simpler than what I was expecting when I first looked at
tar. I agree that while the format is ugly, it is far more common and
the command line is more user friendly.

Cheers,
Rafael


More information about the llvm-commits mailing list