Skip to content

Commit

Permalink
remove temporary file in file transport layer
Browse files Browse the repository at this point in the history
  • Loading branch information
@jonathanpoelen
jonathanpoelen committed Nov 7, 2024
1 parent ea9a87a commit 5bf5fd7
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 152 deletions.
36 changes: 6 additions & 30 deletions src/capture/video_recorder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,39 +93,23 @@ namespace
struct VideoRecorderOutputFile
{
explicit VideoRecorderOutputFile(
std::string_view filename,
char const* filename,
FilePermissions file_permissions,
AclReportApi * acl_report)
: tmp_filename(str_concat(filename, "red-XXXXXX.tmp"_av))
, fd{[
acl_report,
tmp_filename = this->tmp_filename.data(),
mode = file_permissions.permissions_as_uint()
]{
int fd = ::mkostemps(tmp_filename, 4, O_WRONLY | O_CREAT);
: fd{[acl_report, filename, mode = file_permissions.permissions_as_uint()]{
int fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, mode);
if (fd == -1) {
int const errnum = errno;
LOG( LOG_ERR, "can't open temporary file %s : %s [%d]"
, tmp_filename, strerror(errnum), errnum);
LOG( LOG_ERR, "can't open file %s: %s [%d]"
, filename, strerror(errnum), errnum);
video_transport_acl_report(acl_report, errnum);
throw Error(ERR_TRANSPORT_OPEN_FAILED, errnum);
}

if (fchmod(fd, mode) == -1) {
int const errnum = errno;
LOG( LOG_ERR, "can't set file %s mod to u+r, g+r : %s [%d]"
, tmp_filename, strerror(errnum), errnum);
::close(fd);
unlink(tmp_filename);
throw Error(ERR_TRANSPORT_OPEN_FAILED, errnum);
}

return unique_fd{fd};
}()}
, acl_report(acl_report)
, final_filename(filename)
{
::unlink(this->final_filename.c_str());
}

int write(uint8_t const* buf, int buf_size)
Expand Down Expand Up @@ -190,12 +174,6 @@ namespace
{
this->send_remaining_buffer();
this->fd.close();

if (::rename(this->tmp_filename.c_str(), this->final_filename.c_str()) < 0) {
int const errnum = errno;
LOG( LOG_ERR, "renaming file \"%s\" -> \"%s\" failed errno=%d : %s"
, this->tmp_filename, this->final_filename, errnum, strerror(errnum));
}
}

private:
Expand Down Expand Up @@ -224,12 +202,10 @@ namespace

static const int buffer_capacity = 1024*256;

std::string tmp_filename;
unique_fd fd;
int buffer_len = 0;
std::unique_ptr<uint8_t[]> buffer{new uint8_t[buffer_capacity]} /*NOLINT*/;
AclReportApi * acl_report;
std::string final_filename;
};
} // anonymous namespace

Expand Down Expand Up @@ -258,7 +234,7 @@ struct video_recorder::D
std::unique_ptr<uint8_t, default_av_free> custom_io_buffer;
AVIOContext* custom_io_context = nullptr;

D(std::string_view filename, FilePermissions file_permissions, AclReportApi * acl_report)
D(char const* filename, FilePermissions file_permissions, AclReportApi * acl_report)
: out_file(filename, file_permissions, acl_report)
, dst_frame(av_frame_alloc())
, pkt(av_packet_alloc())
Expand Down
73 changes: 22 additions & 51 deletions src/transport/crypto_transport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -663,10 +663,7 @@ OutCryptoTransport::OutCryptoTransport(
, out_file(invalid_fd(), std::move(notify_error))
, cctx(cctx)
, rnd(rnd)
{
this->tmpname[0] = 0;
this->finalname[0] = 0;
}
{}

OutCryptoTransport::~OutCryptoTransport()
{
Expand Down Expand Up @@ -709,59 +706,43 @@ bool OutCryptoTransport::is_open() const
return this->out_file.is_open();
}

void OutCryptoTransport::open(const char * const finalname, const char * const hash_filename, FilePermissions file_permissions, bytes_view derivator)
void OutCryptoTransport::open(std::string&& finalname, std::string&& hash_filename, FilePermissions file_permissions, bytes_view derivator)
{
// This should avoid double open, we do not want that
if (this->is_open()){
LOG(LOG_ERR, "OutCryptoTransport::open (double open error) %s", finalname);
throw Error(ERR_TRANSPORT_OPEN_FAILED);
}
// also ensure pathes are not to long, we will copy them in the object
if (strlen(finalname) >= 2047-15){
LOG(LOG_ERR, "OutCryptoTransport::open finalname oversize");
LOG(LOG_ERR, "OutCryptoTransport::open: double open error: %s", finalname);
throw Error(ERR_TRANSPORT_OPEN_FAILED);
}

// basic compare filename
if (0 == strcmp(finalname, hash_filename)){
LOG(LOG_ERR, "OutCryptoTransport::open finalname and hash_filename are same");
if (finalname == hash_filename){
LOG(LOG_ERR, "OutCryptoTransport::open: finalname and hash_filename are same");
throw Error(ERR_TRANSPORT_OPEN_FAILED);
}
snprintf(this->tmpname, sizeof(this->tmpname), "%sred-XXXXXX.tmp", finalname);
this->out_file.open(unique_fd(::mkostemps(this->tmpname, 4, O_WRONLY | O_CREAT)));
if (not this->is_open()){
int const err = errno;
LOG(LOG_ERR, "OutCryptoTransport::open : open failed (%s -> %s): %s", this->tmpname, finalname, strerror(errno));
throw Error(ERR_TRANSPORT_OPEN_FAILED, err);
}
if (chmod(this->tmpname, file_permissions.permissions_as_uint()) == -1) {
int const err = errno;

LOG( LOG_ERR, "can't set file %s mod to %o : %s [%d]"
, this->tmpname
, file_permissions.permissions_as_uint()
, strerror(err)
, err);
LOG(LOG_INFO, "OutCryptoTransport::open : chmod failed (%s -> %s)", this->tmpname, finalname);
auto const mode = file_permissions.permissions_as_uint();
this->out_file.open(unique_fd(finalname.c_str(), O_WRONLY | O_CREAT | O_EXCL, mode));
if (not this->out_file.is_open()){
int const err = errno;
LOG(LOG_ERR, "OutCryptoTransport::open: open failed (%s): %s", finalname, strerror(err));
throw Error(ERR_TRANSPORT_OPEN_FAILED, err);
}

if (!utils::strbcpy(this->finalname, finalname)) {
LOG(LOG_ERR, "OutCryptoTransport::open finalname too long");
throw Error(ERR_TRANSPORT_OPEN_FAILED);
}
this->hash_filename = hash_filename;
this->finalname = std::move(finalname);
this->hash_filename = std::move(hash_filename);
this->derivator.assign(derivator.begin(), derivator.end());

ocrypto::Result res = this->encrypter.open(derivator);
this->out_file.send(res.buf);
}

// derivator implicitly basename(finalname)
void OutCryptoTransport::open(const char * finalname, const char * const hash_filename, FilePermissions file_permissions)
void OutCryptoTransport::open(std::string&& finalname, std::string&& hash_filename, FilePermissions file_permissions)
{
size_t base_len = 0;
const char * base = basename_len(finalname, base_len);
this->open(finalname, hash_filename, file_permissions, {base, base_len});
const char * base = basename_len(finalname.c_str(), base_len);
const auto derivator = bytes_view{base, base_len};
this->open(std::move(finalname), std::move(hash_filename), file_permissions, derivator);
}

void OutCryptoTransport::close(HashArray & qhash, HashArray & fhash)
Expand All @@ -779,15 +760,6 @@ void OutCryptoTransport::close(HashArray & qhash, HashArray & fhash)
const ocrypto::Result res = this->encrypter.close(qhash, fhash);
this->out_file.send(res.buf);
this->out_file.close();
if (this->tmpname[0] != 0){
if (::rename(this->tmpname, this->finalname) < 0) {
int const err = errno;
LOG(LOG_ERR, "OutCryptoTransport::close Renaming file \"%s\" -> \"%s\" failed, errno=%d : %s"
, this->tmpname, this->finalname, err, strerror(err));
throw Error(ERR_TRANSPORT_WRITE_FAILED, err);
}
this->tmpname[0] = 0;
}
this->create_hash_file(qhash, fhash);
}

Expand Down Expand Up @@ -821,6 +793,7 @@ void OutCryptoTransport::create_hash_file(HashArray const & qhash, HashArray con

unique_fd open_file = open_hash();

// error -> create subdirectory then re-open
if(!open_file.is_open() && errno == ENOENT) {
std::string dir_path = this->hash_filename.substr(0, this->hash_filename.find_last_of('/'));
if (!dir_path.empty()) {
Expand All @@ -837,7 +810,7 @@ void OutCryptoTransport::create_hash_file(HashArray const & qhash, HashArray con
}

struct stat stat;
if (-1 == ::stat(this->finalname, &stat)) {
if (-1 == ::stat(this->finalname.c_str(), &stat)) {
int const err = errno;
LOG(LOG_ERR, "Failed writing signature to hash file %s [err %d]",
this->hash_filename, err);
Expand All @@ -857,7 +830,7 @@ void OutCryptoTransport::create_hash_file(HashArray const & qhash, HashArray con
char extension[256] = {};

canonical_path(
this->finalname,
this->finalname.c_str(),
path, sizeof(path),
basename, sizeof(basename),
extension, sizeof(extension)
Expand Down Expand Up @@ -885,7 +858,7 @@ void OutCryptoTransport::create_hash_file(HashArray const & qhash, HashArray con
void OutCryptoTransport::do_send(const uint8_t * data, size_t len)
{
if (not this->out_file.is_open()){
LOG(LOG_ERR, "OutCryptoTransport::do_send failed: file not opened (%s->%s)", this->tmpname, this->finalname);
LOG(LOG_ERR, "OutCryptoTransport::do_send failed: file not opened (%s)", this->finalname);
throw Error(ERR_TRANSPORT_WRITE_FAILED);
}
send_data(data, len, this->encrypter, this->out_file);
Expand All @@ -895,9 +868,7 @@ bool OutCryptoTransport::cancel()
{
if (this->is_open()) {
this->out_file.close();
if (this->tmpname[0] != 0){
return ::remove(this->tmpname) == 0;
}
return ::remove(this->finalname.c_str()) == 0;
}
return true;
}
Expand Down
9 changes: 4 additions & 5 deletions src/transport/crypto_transport.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class OutCryptoTransport : public Transport
FileTransport::ErrorNotifier notify_error
) noexcept;

[[nodiscard]] const char * get_finalname() const noexcept { return this->finalname; }
[[nodiscard]] const char * get_finalname() const noexcept { return this->finalname.c_str(); }

~OutCryptoTransport();

Expand All @@ -183,10 +183,10 @@ class OutCryptoTransport : public Transport

[[nodiscard]] bool is_open() const;

void open(const char * const finalname, const char * const hash_filename, FilePermissions file_permissions, bytes_view derivator);
void open(std::string&& finalname, std::string&& hash_filename, FilePermissions file_permissions, bytes_view derivator);

// derivator implicitly basename(finalname)
void open(const char * finalname, const char * const hash_filename, FilePermissions file_permissions);
void open(std::string&& finalname, std::string&& hash_filename, FilePermissions file_permissions);

void close(HashArray & qhash, HashArray & fhash);

Expand All @@ -199,8 +199,7 @@ class OutCryptoTransport : public Transport
private:
ocrypto encrypter;
OutFileTransport out_file;
char tmpname[2048];
char finalname[2048];
std::string finalname;
std::string hash_filename;
CryptoContext & cctx;
Random & rnd;
Expand Down
52 changes: 7 additions & 45 deletions src/transport/out_filename_sequence_transport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ OutFilenameSequenceTransport::FilenameGenerator::FilenameGenerator(
const char * const prefix,
const char * const filename,
const char * const extension)
: last_filename(nullptr)
, last_num(-1u)
: last_num(-1u)
{
int len = snprintf(this->filename_gen, sizeof(this->filename_gen), "%s%s-", prefix, filename);
if (len <= 0
Expand All @@ -46,8 +45,8 @@ OutFilenameSequenceTransport::FilenameGenerator::FilenameGenerator(

const char * OutFilenameSequenceTransport::FilenameGenerator::get(unsigned count) const
{
if (count == this->last_num && this->last_filename) {
return this->last_filename;
if (count == this->last_num) {
return this->filename_gen;
}

snprintf( this->filename_gen + this->filename_suffix_pos
Expand All @@ -56,12 +55,6 @@ const char * OutFilenameSequenceTransport::FilenameGenerator::get(unsigned count
return this->filename_gen;
}

void OutFilenameSequenceTransport::FilenameGenerator::set_last_filename(unsigned num, const char * name)
{
this->last_num = num;
this->last_filename = name;
}


OutFilenameSequenceTransport::OutFilenameSequenceTransport(
const char * const prefix,
Expand All @@ -73,7 +66,6 @@ OutFilenameSequenceTransport::OutFilenameSequenceTransport(
, buf_(invalid_fd(), std::move(notify_error))
, file_permissions_(file_permissions)
{
this->current_filename_[0] = 0;
}

char const* OutFilenameSequenceTransport::seqgen(unsigned i) const noexcept
Expand Down Expand Up @@ -121,8 +113,8 @@ int OutFilenameSequenceTransport::do_next()
{
if (this->buf_.is_open()) {
this->buf_.close();
// LOG(LOG_INFO, "pngcapture: \"%s\" -> \"%s\".", this->current_filename_, this->rename_to);
return this->rename_filename() ? 0 : 1;
++this->num_file_;
return 0;
}

return 1;
Expand All @@ -131,40 +123,10 @@ int OutFilenameSequenceTransport::do_next()
void OutFilenameSequenceTransport::open_filename()
{
const char * filename = this->filegen_.get(this->num_file_);
snprintf(this->current_filename_, sizeof(this->current_filename_),
"%sred-XXXXXX.tmp", filename);
const int fd = ::mkostemps(this->current_filename_, 4, O_WRONLY | O_CREAT);
const auto mode = file_permissions_.permissions_as_uint();
const int fd = ::open(filename, O_WRONLY | O_CREAT | O_TRUNC, mode);
if (fd < 0) {
throw Error(ERR_TRANSPORT_OPEN_FAILED, errno);
}
// LOG(LOG_INFO, "pngcapture=%s", this->current_filename_);
// TODO PERF used fchmod
if (chmod(this->current_filename_, file_permissions_.permissions_as_uint()) == -1) {
LOG( LOG_ERR, "can't set file %s mod to u+r, g+r : %s [%d]"
, this->current_filename_, strerror(errno), errno);
}
this->filegen_.set_last_filename(this->num_file_, this->current_filename_);
this->buf_.open(unique_fd{fd});
}

const char * OutFilenameSequenceTransport::rename_filename()
{
this->filegen_.set_last_filename(-1u, "");
const char * filename = this->filegen_.get(this->num_file_);
this->filegen_.set_last_filename(this->num_file_, this->current_filename_);

const int res = ::rename(this->current_filename_, filename);
// LOG( LOG_INFO, "renaming file \"%s\" to \"%s\""
// , this->current_filename_, filename);
if (res < 0) {
LOG( LOG_ERR, "renaming file \"%s\" -> \"%s\" failed error=%d : %s"
, this->current_filename_, filename, errno, strerror(errno));
return nullptr;
}

this->current_filename_[0] = 0;
++this->num_file_;
this->filegen_.set_last_filename(-1u, "");

return filename;
}
7 changes: 0 additions & 7 deletions src/transport/out_filename_sequence_transport.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ class OutFilenameSequenceTransport : public SequencedTransport
char extension[12];
mutable char filename_gen[2070];
std::size_t filename_suffix_pos;

const char * last_filename;
unsigned last_num;

public:
Expand All @@ -66,8 +64,6 @@ class OutFilenameSequenceTransport : public SequencedTransport
const char * const extension);

const char * get(unsigned count) const;

void set_last_filename(unsigned num, const char * name);
};

void do_send(const uint8_t * data, size_t len) override;
Expand All @@ -76,9 +72,6 @@ class OutFilenameSequenceTransport : public SequencedTransport
int do_next();
void open_filename();

const char * rename_filename();

char current_filename_[1024];
FilenameGenerator filegen_;
OutFileTransport buf_;
FilePermissions file_permissions_;
Expand Down
Loading

0 comments on commit 5bf5fd7

Please sign in to comment.