Skip to content

Commit

Permalink
👷 gucc: rewrite pacmanconf to reuse file content
Browse files Browse the repository at this point in the history
don't make extra memory allocations:
- don't allocate vector of strings to find locations
- don't convert that vector of strings into new string

previously:
```
heaptrack stats:
	allocations:          	69
	leaked allocations:   	0
	temporary allocations:	4
```

after:
```
heaptrack stats:
	allocations:          	32
	leaked allocations:   	0
	temporary allocations:	5
```
  • Loading branch information
vnepogodin committed Jul 3, 2024
1 parent 1e6bc8b commit b6d5c27
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 17 deletions.
36 changes: 21 additions & 15 deletions gucc/src/pacmanconf_repo.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "gucc/pacmanconf_repo.hpp"
#include "gucc/file_utils.hpp"
#include "gucc/string_utils.hpp"

#include <fstream> // for ofstream
#include <string> // for string
Expand All @@ -17,9 +18,9 @@
#pragma GCC diagnostic ignored "-Wold-style-cast"
#endif

#include <range/v3/algorithm/find_if.hpp>
#include <range/v3/range/conversion.hpp>
#include <range/v3/view/filter.hpp>
#include <range/v3/view/join.hpp>
#include <range/v3/view/split.hpp>

#if defined(__clang__)
Expand All @@ -31,33 +32,38 @@
namespace gucc::detail::pacmanconf {

bool push_repos_front(std::string_view file_path, std::string_view value) noexcept {

Check failure on line 34 in gucc/src/pacmanconf_repo.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/gucc/src/pacmanconf_repo.cpp:34:6 [modernize-use-trailing-return-type

use a trailing return type for this function

Check failure on line 34 in gucc/src/pacmanconf_repo.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/gucc/src/pacmanconf_repo.cpp:34:23 [bugprone-easily-swappable-parameters

2 adjacent parameters of 'push_repos_front' of similar type ('std::string_view') are easily swapped by mistake
using StringVec = std::vector<std::string>;
auto&& file_content = file_utils::read_whole_file(file_path);
if (file_content.empty()) {
spdlog::error("[PACMANCONFREPO] '{}' error occurred!", file_path);
return false;
}
auto&& content_lines = file_content | ranges::views::split('\n') | ranges::to<std::vector<std::string>>();
for (std::size_t i = 1; i < content_lines.size(); ++i) {
const std::string_view current_line{content_lines[i - 1]};
if (current_line.empty() || current_line.starts_with('#') || !current_line.starts_with('[') || current_line.starts_with("[options]")) {
continue;
}

auto&& to_be_inserted = value | ranges::views::split('\n') | ranges::to<std::vector<std::string>>();
to_be_inserted.emplace_back("\n");
content_lines.insert(content_lines.begin() + static_cast<StringVec::iterator::difference_type>(i - 1),
std::move_iterator(to_be_inserted.begin()), std::move_iterator(to_be_inserted.end()));
break;
auto file_split_view = utils::make_split_view(file_content);
// Find the insertion point (using ranges for iteration)
auto insertion_point_it = ranges::find_if(
file_split_view,
[](auto&& rng) {
auto&& line = std::string_view(&*rng.begin(), static_cast<size_t>(ranges::distance(rng)));
return !line.empty() && !line.starts_with('#') && line.starts_with('[') && !line.starts_with("[options]");
});

// Handle case where insertion point is not found
if (insertion_point_it == ranges::end(file_split_view)) {
// No suitable insertion point found
spdlog::error("[PACMANCONFREPO] Could not find a suitable insertion point in {}", file_path);
return false;
}

auto line = *insertion_point_it;
auto pos = file_content.find(line);
file_content.insert(pos - 1, std::string{'\n'} + std::string{value} + std::string{'\n'});

std::ofstream pacmanconf_file{file_path.data()};
if (!pacmanconf_file.is_open()) {
spdlog::error("[PACMANCONFREPO] '{}' open failed: {}", file_path, std::strerror(errno));
return false;
}
std::string&& result = content_lines | ranges::views::join('\n') | ranges::to<std::string>();
pacmanconf_file << result;
pacmanconf_file << file_content;
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions gucc/tests/unit-pacmanconf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ ParallelDownloads = 10
[cachyos]
Include = /etc/pacman.d/cachyos-mirrorlist
[core]
Include = /etc/pacman.d/mirrorlist
Expand All @@ -115,7 +114,8 @@ Include = /etc/pacman.d/mirrorlist
#Include = /etc/pacman.d/mirrorlist
[multilib]
Include = /etc/pacman.d/mirrorlist)";
Include = /etc/pacman.d/mirrorlist
)";

int main() {

Check failure on line 120 in gucc/tests/unit-pacmanconf.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/gucc/tests/unit-pacmanconf.cpp:120:5 [modernize-use-trailing-return-type

use a trailing return type for this function
using namespace gucc;

Check failure on line 121 in gucc/tests/unit-pacmanconf.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/gucc/tests/unit-pacmanconf.cpp:121:5 [google-build-using-namespace

do not use namespace using-directives; use using-declarations instead
Expand Down

0 comments on commit b6d5c27

Please sign in to comment.