Skip to content

Commit 021de7c

Browse files
committed
[llvm-objcopy][NFC] Move ownership keeping code into restoreStatOnFile().
The D93881 added functionality which preserve ownership for output file if llvm-objcopy is called under root. That code was added into the place where output file is created. The llvm-objcopy already has a function which sets/restores rights/permissions for the output file. That is the restoreStatOnFile() function. This patch moves code (preserving ownershipping) into the restoreStatOnFile() function. Differential Revision: https://reviews.llvm.org/D98511
1 parent d9ef6bc commit 021de7c

File tree

4 files changed

+18
-44
lines changed

4 files changed

+18
-44
lines changed

llvm/include/llvm/Support/FileOutputBuffer.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@ class FileOutputBuffer {
3434
/// Don't use mmap and instead write an in-memory buffer to a file when this
3535
/// buffer is closed.
3636
F_no_mmap = 2,
37-
38-
/// Preserve ownership if the file already exists.
39-
F_keep_ownership = 4,
4037
};
4138

4239
/// Factory method to create an OutputBuffer object which manages a read/write
@@ -49,8 +46,7 @@ class FileOutputBuffer {
4946
/// \p Size. It is an error to specify F_modify and Size=-1 if \p FilePath
5047
/// does not exist.
5148
static Expected<std::unique_ptr<FileOutputBuffer>>
52-
create(StringRef FilePath, size_t Size, unsigned Flags = 0,
53-
unsigned UserID = 0, unsigned GroupID = 0);
49+
create(StringRef FilePath, size_t Size, unsigned Flags = 0);
5450

5551
/// Returns a pointer to the start of the buffer.
5652
virtual uint8_t *getBufferStart() const = 0;

llvm/lib/Support/FileOutputBuffer.cpp

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -125,22 +125,14 @@ createInMemoryBuffer(StringRef Path, size_t Size, unsigned Mode) {
125125
}
126126

127127
static Expected<std::unique_ptr<FileOutputBuffer>>
128-
createOnDiskBuffer(StringRef Path, size_t Size, unsigned Mode,
129-
bool KeepOwnership, unsigned UserID, unsigned GroupID) {
128+
createOnDiskBuffer(StringRef Path, size_t Size, unsigned Mode) {
130129
Expected<fs::TempFile> FileOrErr =
131130
fs::TempFile::create(Path + ".tmp%%%%%%%", Mode);
132131
if (!FileOrErr)
133132
return FileOrErr.takeError();
134133
fs::TempFile File = std::move(*FileOrErr);
135134

136135
#ifndef _WIN32
137-
// Try to preserve file ownership if requested.
138-
if (KeepOwnership) {
139-
fs::file_status Stat;
140-
if (!fs::status(File.FD, Stat) && Stat.getUser() == 0)
141-
fs::changeFileOwnership(File.FD, UserID, GroupID);
142-
}
143-
144136
// On Windows, CreateFileMapping (the mmap function on Windows)
145137
// automatically extends the underlying file. We don't need to
146138
// extend the file beforehand. _chsize (ftruncate on Windows) is
@@ -171,8 +163,7 @@ createOnDiskBuffer(StringRef Path, size_t Size, unsigned Mode,
171163

172164
// Create an instance of FileOutputBuffer.
173165
Expected<std::unique_ptr<FileOutputBuffer>>
174-
FileOutputBuffer::create(StringRef Path, size_t Size, unsigned Flags,
175-
unsigned UserID, unsigned GroupID) {
166+
FileOutputBuffer::create(StringRef Path, size_t Size, unsigned Flags) {
176167
// Handle "-" as stdout just like llvm::raw_ostream does.
177168
if (Path == "-")
178169
return createInMemoryBuffer("-", Size, /*Mode=*/0);
@@ -205,8 +196,7 @@ FileOutputBuffer::create(StringRef Path, size_t Size, unsigned Flags,
205196
if (Flags & F_no_mmap)
206197
return createInMemoryBuffer(Path, Size, Mode);
207198
else
208-
return createOnDiskBuffer(Path, Size, Mode, Flags & F_keep_ownership,
209-
UserID, GroupID);
199+
return createOnDiskBuffer(Path, Size, Mode);
210200
default:
211201
return createInMemoryBuffer(Path, Size, Mode);
212202
}

llvm/tools/llvm-objcopy/llvm-objcopy.cpp

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ namespace llvm {
5858
namespace objcopy {
5959

6060
Error writeToFile(StringRef OutputFileName,
61-
std::function<Error(raw_ostream &)> Write, bool KeepOwnership,
62-
unsigned UserID, unsigned GroupID) {
61+
std::function<Error(raw_ostream &)> Write) {
6362
if (OutputFileName == "-")
6463
return Write(outs());
6564

@@ -74,15 +73,6 @@ Error writeToFile(StringRef OutputFileName,
7473
if (!Temp)
7574
return createFileError(OutputFileName, Temp.takeError());
7675

77-
#ifndef _WIN32
78-
// Try to preserve file ownership if requested.
79-
if (KeepOwnership) {
80-
sys::fs::file_status Stat;
81-
if (!sys::fs::status(Temp->FD, Stat) && Stat.getUser() == 0)
82-
sys::fs::changeFileOwnership(Temp->FD, UserID, GroupID);
83-
}
84-
#endif
85-
8676
raw_fd_ostream Out(Temp->FD, false);
8777

8878
if (Error E = Write(Out)) {
@@ -156,9 +146,9 @@ static Error deepWriteArchive(StringRef ArcName,
156146
// now in-memory buffers can not be completely avoided since
157147
// NewArchiveMember still requires them even though writeArchive does not
158148
// write them on disk.
159-
Expected<std::unique_ptr<FileOutputBuffer>> FB = FileOutputBuffer::create(
160-
Member.MemberName, Member.Buf->getBufferSize(),
161-
FileOutputBuffer::F_executable | FileOutputBuffer::F_keep_ownership);
149+
Expected<std::unique_ptr<FileOutputBuffer>> FB =
150+
FileOutputBuffer::create(Member.MemberName, Member.Buf->getBufferSize(),
151+
FileOutputBuffer::F_executable);
162152
if (!FB)
163153
return FB.takeError();
164154
std::copy(Member.Buf->getBufferStart(), Member.Buf->getBufferEnd(),
@@ -306,6 +296,12 @@ static Error restoreStatOnFile(StringRef Filename,
306296
if (auto EC = sys::fs::setPermissions(FD, Perm))
307297
#endif
308298
return createFileError(Filename, EC);
299+
300+
#ifndef _WIN32
301+
// Keep ownership if llvm-objcopy is called under root.
302+
if (Config.InputFilename == Config.OutputFilename && OStat.getUser() == 0)
303+
sys::fs::changeFileOwnership(FD, Stat.getUser(), Stat.getGroup());
304+
#endif
309305
}
310306

311307
if (auto EC = sys::Process::SafelyCloseFileDescriptor(FD))
@@ -360,14 +356,10 @@ static Error executeObjcopy(CopyConfig &Config) {
360356
return E;
361357
} else {
362358
if (Error E = writeToFile(
363-
Config.OutputFilename,
364-
[&](raw_ostream &OutFile) -> Error {
359+
Config.OutputFilename, [&](raw_ostream &OutFile) -> Error {
365360
return executeObjcopyOnBinary(
366361
Config, *BinaryOrErr.get().getBinary(), OutFile);
367-
},
368-
Config.InputFilename != "-" &&
369-
Config.InputFilename == Config.OutputFilename,
370-
Stat.getUser(), Stat.getGroup()))
362+
}))
371363
return E;
372364
}
373365
}

llvm/tools/llvm-objcopy/llvm-objcopy.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,9 @@ createNewArchiveMembers(CopyConfig &Config, const object::Archive &Ar);
3131
/// \p OutputFileName: std::outs for the "-", raw_null_ostream for
3232
/// the "/dev/null", temporary file in the same directory as the final output
3333
/// file for other names. The final output file is atomically replaced with
34-
/// the temporary file after \p Write handler is finished. \p KeepOwnership
35-
/// used to setting specified \p UserID and \p GroupID for the resulting file
36-
/// if writeToFile is called under /root.
34+
/// the temporary file after \p Write handler is finished.
3735
Error writeToFile(StringRef OutputFileName,
38-
std::function<Error(raw_ostream &)> Write,
39-
bool KeepOwnership = false, unsigned UserID = 0,
40-
unsigned GroupID = 0);
36+
std::function<Error(raw_ostream &)> Write);
4137

4238
} // end namespace objcopy
4339
} // end namespace llvm

0 commit comments

Comments
 (0)