Skip to content

Commit

Permalink
Add a new overload to locate and replace multiOpen with locate (#1066)
Browse files Browse the repository at this point in the history
There is a risk that multiOpen open too many files at the same time and
use all the fd. Should prefer the new locate over multiOpen. (#1052)
  • Loading branch information
wengxt authored May 25, 2024
1 parent 25d7c5c commit a1e7502
Show file tree
Hide file tree
Showing 14 changed files with 267 additions and 75 deletions.
33 changes: 29 additions & 4 deletions src/frontend/ibusfrontend/ibusfrontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,45 @@
#include <sys/stat.h>
#include <sys/wait.h>
#include <unistd.h>
#include <cerrno>
#include <csignal>
#include <cstddef>
#include <cstdint>
#include <cstdio>
#include <cstdlib>
#include <memory>
#include <optional>
#include <set>
#include <string>
#include <tuple>
#include <utility>
#include <vector>
#include <fmt/core.h>
#include <fmt/format.h>
#include "fcitx-config/iniparser.h"
#include "fcitx-config/rawconfig.h"
#include "fcitx-utils/capabilityflags.h"
#include "fcitx-utils/dbus/bus.h"
#include "fcitx-utils/dbus/message.h"
#include "fcitx-utils/dbus/objectvtable.h"
#include "fcitx-utils/dbus/servicewatcher.h"
#include "fcitx-utils/dbus/variant.h"
#include "fcitx-utils/event.h"
#include "fcitx-utils/flags.h"
#include "fcitx-utils/handlertable.h"
#include "fcitx-utils/key.h"
#include "fcitx-utils/log.h"
#include "fcitx-utils/metastring.h"
#include "fcitx-utils/macros.h"
#include "fcitx-utils/misc.h"
#include "fcitx-utils/rect.h"
#include "fcitx-utils/standardpath.h"
#include "fcitx-utils/stringutils.h"
#include "fcitx-utils/textformatflags.h"
#include "fcitx-utils/utf8.h"
#include "fcitx-utils/uuid_p.h"
#include "fcitx/addonfactory.h"
#include "fcitx/addoninstance.h"
#include "fcitx/event.h"
#include "fcitx/inputcontext.h"
#include "fcitx/inputpanel.h"
#include "fcitx/instance.h"
Expand Down Expand Up @@ -634,8 +659,8 @@ std::set<std::string> allSocketPaths(const StandardPath &standardPath) {
if (isInFlatpak()) {
// Flatpak always use DISPLAY=:99, which means we will need to guess
// what files are available.
auto map = standardPath.multiOpenFilter(
StandardPath::Type::Config, "ibus/bus", O_RDONLY,
auto map = standardPath.locateWithFilter(
StandardPath::Type::Config, "ibus/bus",
[](const std::string &path, const std::string &, bool user) {
if (!user) {
return false;
Expand All @@ -644,7 +669,7 @@ std::set<std::string> allSocketPaths(const StandardPath &standardPath) {
});

for (const auto &item : map) {
paths.insert(item.second.path());
paths.insert(item.second);
}

// Make the guess that display is 0, it is the most common value that
Expand Down
7 changes: 5 additions & 2 deletions src/lib/fcitx-config/configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@
#ifndef _FCITX_CONFIG_CONFIGURATION_H_
#define _FCITX_CONFIG_CONFIGURATION_H_

#include <memory>
#include <string>
#include <vector>
#include <fcitx-config/option.h>
#include <fcitx-config/optiontypename.h>
#include <fcitx-config/rawconfig.h>
#include <fcitx-utils/macros.h>
#include "fcitxconfig_export.h"

#include <memory>

#define FCITX_CONFIGURATION_EXTEND(NAME, SUBCLASS, ...) \
class NAME; \
FCITX_SPECIALIZE_TYPENAME(NAME, #NAME) \
Expand Down
16 changes: 10 additions & 6 deletions src/lib/fcitx-config/iniparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
#include "iniparser.h"
#include <fcntl.h>
#include <cstdio>
#include <functional>
#include <string>
#include <string_view>
#include "fcitx-config/rawconfig.h"
#include "fcitx-utils/fs.h"
#include "fcitx-utils/macros.h"
#include "fcitx-utils/misc.h"
#include "fcitx-utils/standardpath.h"
#include "fcitx-utils/stringutils.h"
#include "fcitx-utils/unixfd.h"
Expand Down Expand Up @@ -54,8 +60,6 @@ void readFromIni(RawConfig &config, FILE *fin) {
continue;
}

std::string::size_type equalPos;

if (lineBuf.front() == '[' && lineBuf.back() == ']') {
currentGroup = lineBuf.substr(1, lineBuf.size() - 2);
config.visitItemsOnPath(
Expand All @@ -65,8 +69,8 @@ void readFromIni(RawConfig &config, FILE *fin) {
}
},
currentGroup);
} else if ((equalPos = lineBuf.find_first_of('=')) !=
std::string::npos) {
} else if (std::string::size_type equalPos = lineBuf.find_first_of('=');
equalPos != std::string::npos) {
auto name = lineBuf.substr(0, equalPos);
auto valueStart = equalPos + 1;

Expand All @@ -91,7 +95,7 @@ void readFromIni(RawConfig &config, FILE *fin) {
}
}

bool writeAsIni(const RawConfig &root, FILE *fout) {
bool writeAsIni(const RawConfig &config, FILE *fout) {
std::function<bool(const RawConfig &, const std::string &path)> callback;

callback = [fout, &callback](const RawConfig &config,
Expand Down Expand Up @@ -131,7 +135,7 @@ bool writeAsIni(const RawConfig &root, FILE *fout) {
return true;
};

return callback(root, "");
return callback(config, "");
}

void readAsIni(RawConfig &rawConfig, const std::string &path) {
Expand Down
33 changes: 19 additions & 14 deletions src/lib/fcitx-config/iniparser.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
#ifndef _FCITX_CONFIG_INIPARSER_H_
#define _FCITX_CONFIG_INIPARSER_H_

#include <cstdio>
#include <string>
#include <fcitx-config/rawconfig.h>
#include <fcitx-utils/standardpath.h>
#include "fcitxconfig_export.h"
Expand All @@ -17,22 +19,25 @@ FCITXCONFIG_EXPORT void readFromIni(RawConfig &config, int fd);
FCITXCONFIG_EXPORT bool writeAsIni(const RawConfig &config, int fd);
FCITXCONFIG_EXPORT void readFromIni(RawConfig &config, FILE *fin);
FCITXCONFIG_EXPORT bool writeAsIni(const RawConfig &config, FILE *fout);
FCITXCONFIG_EXPORT void readAsIni(Configuration &, const std::string &name);
FCITXCONFIG_EXPORT void readAsIni(RawConfig &, const std::string &name);
FCITXCONFIG_EXPORT void readAsIni(Configuration &, StandardPath::Type type,
const std::string &name);
FCITXCONFIG_EXPORT void readAsIni(RawConfig &, StandardPath::Type type,
const std::string &name);
FCITXCONFIG_EXPORT bool safeSaveAsIni(const Configuration &,
const std::string &name);
FCITXCONFIG_EXPORT bool safeSaveAsIni(const RawConfig &,
const std::string &name);
FCITXCONFIG_EXPORT bool safeSaveAsIni(const Configuration &,
FCITXCONFIG_EXPORT void readAsIni(Configuration &configuration,
const std::string &path);
FCITXCONFIG_EXPORT void readAsIni(RawConfig &rawConfig,
const std::string &path);
FCITXCONFIG_EXPORT void readAsIni(Configuration &configuration,
StandardPath::Type type,
const std::string &path);
FCITXCONFIG_EXPORT void readAsIni(RawConfig &rawConfig, StandardPath::Type type,
const std::string &path);
FCITXCONFIG_EXPORT bool safeSaveAsIni(const Configuration &configuration,
const std::string &path);
FCITXCONFIG_EXPORT bool safeSaveAsIni(const RawConfig &rawConfig,
const std::string &path);
FCITXCONFIG_EXPORT bool safeSaveAsIni(const Configuration &configuration,
StandardPath::Type type,
const std::string &name);
FCITXCONFIG_EXPORT bool safeSaveAsIni(const RawConfig &,
const std::string &path);
FCITXCONFIG_EXPORT bool safeSaveAsIni(const RawConfig &rawConfig,
StandardPath::Type type,
const std::string &name);
const std::string &path);
} // namespace fcitx

#endif // _FCITX_CONFIG_INIPARSER_H_
6 changes: 4 additions & 2 deletions src/lib/fcitx-config/optiontypename.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@
#define _FCITX_CONFIG_TYPENAME_H_

#include <string>
#include <type_traits>
#include <vector>
#include <fcitx-utils/color.h>
#include <fcitx-utils/i18nstring.h>
#include <fcitx-utils/key.h>
#include <fcitx-utils/macros.h>
#include <fcitx-utils/semver.h>

namespace fcitx {
Expand Down Expand Up @@ -42,8 +45,7 @@ struct OptionTypeName<std::vector<T>> {
};

template <typename T>
struct OptionTypeName<T,
typename std::enable_if<std::is_enum<T>::value>::type> {
struct OptionTypeName<T, std::enable_if_t<std::is_enum_v<T>>> {
static std::string get() { return "Enum"; }
};
} // namespace fcitx
Expand Down
42 changes: 39 additions & 3 deletions src/lib/fcitx-utils/standardpath.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,30 @@
#include <dirent.h>
#include <fcntl.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#include <algorithm>
#include <atomic>
#include <cstdint>
#include <cstdio>
#include <cstdlib>
#include <cstring>
#include <exception>
#include <functional>
#include <map>
#include <memory>
#include <stdexcept>
#include <string>
#include <string_view>
#include <tuple>
#include <unordered_map>
#include <unordered_set>
#include <utility>
#include <vector>
#include "config.h"
#include "fs.h"
#include "macros.h"
#include "misc.h"
#include "misc_p.h"
#include "stringutils.h"

Expand Down Expand Up @@ -594,10 +608,11 @@ StandardPath::openUserTemp(Type type, const std::string &pathOrig) const {
fullPathOrig = constructPath(dirPath, pathOrig);
}
if (fs::makePath(fs::dirName(fullPath))) {
auto cPath = makeUniqueCPtr(strdup(fullPath.c_str()));
int fd = mkstemp(cPath.get());
std::vector<char> cPath(fullPath.data(),
fullPath.data() + fullPath.size() + 1);
int fd = mkstemp(cPath.data());
if (fd >= 0) {
return {fd, fullPathOrig, cPath.get()};
return {fd, fullPathOrig, cPath.data()};
}
}
return {};
Expand All @@ -623,6 +638,27 @@ bool StandardPath::safeSave(Type type, const std::string &pathOrig,
return false;
}

std::map<std::string, std::string> StandardPath::locateWithFilter(
Type type, const std::string &path,
std::function<bool(const std::string &path, const std::string &dir,
bool user)>
filter) const {
std::map<std::string, std::string> result;
scanFiles(type, path,
[&result, &filter](const std::string &path,
const std::string &dir, bool isUser) {
if (!result.count(path) && filter(path, dir, isUser)) {
auto fullPath = constructPath(dir, path);
if (fs::isreg(fullPath)) {
result.emplace(path, std::move(fullPath));
}
}
return true;
});

return result;
}

StandardPathFileMap StandardPath::multiOpenFilter(
Type type, const std::string &path, int flags,
std::function<bool(const std::string &path, const std::string &dir,
Expand Down
47 changes: 43 additions & 4 deletions src/lib/fcitx-utils/standardpath.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@
/// \endcode
/// Open all files under $XDG_CONFIG_{HOME,DIRS}/fcitx5/inputmethod/*.conf.

#include <cstdint>
#include <functional>
#include <map>
#include <memory>
#include <string>
#include <unordered_map>
#include <vector>
#include <fcitx-utils/log.h>
#include <fcitx-utils/macros.h>
#include <fcitx-utils/stringutils.h>
#include <fcitx-utils/unixfd.h>
Expand All @@ -40,7 +43,8 @@ class Chainer;
template <>
class Chainer<> {
public:
bool operator()(const std::string &, const std::string &, bool) {
bool operator()(const std::string & /*unused*/,
const std::string & /*unused*/, bool /*unused*/) {
return true;
}
};
Expand Down Expand Up @@ -86,7 +90,8 @@ NotFilter<T> Not(T t) {

/// \brief Filter class that filters based on user file.
struct FCITXUTILS_EXPORT User {
bool operator()(const std::string &, const std::string &, bool isUser) {
bool operator()(const std::string & /*unused*/,
const std::string & /*unused*/, bool isUser) {
return isUser;
}
};
Expand All @@ -95,7 +100,8 @@ struct FCITXUTILS_EXPORT User {
struct FCITXUTILS_EXPORT Prefix {
Prefix(const std::string &prefix_) : prefix(prefix_) {}

bool operator()(const std::string &path, const std::string &, bool) const {
bool operator()(const std::string &path, const std::string & /*unused*/,
bool /*unused*/) const {
return stringutils::startsWith(path, prefix);
}

Expand All @@ -106,7 +112,8 @@ struct FCITXUTILS_EXPORT Prefix {
struct FCITXUTILS_EXPORT Suffix {
Suffix(const std::string &suffix_) : suffix(suffix_) {}

bool operator()(const std::string &path, const std::string &, bool) const {
bool operator()(const std::string &path, const std::string & /*unused*/,
bool /*unused*/) const {
return stringutils::endsWith(path, suffix);
}

Expand Down Expand Up @@ -297,9 +304,41 @@ class FCITXUTILS_EXPORT StandardPath {
bool safeSave(Type type, const std::string &pathOrig,
const std::function<bool(int)> &callback) const;

/**
* \brief Locate all files match the filter under first [directory]/[path].
*
* Prefer this function over multiOpenFilter, if there could be too many
* files that exceeds the systems file descriptor limit.
* @since 5.1.10
* @see multiOpenFilter
*/
std::map<std::string, std::string>
locateWithFilter(Type type, const std::string &path,
std::function<bool(const std::string &path,
const std::string &dir, bool user)>
filter) const;

/**
* \brief Locate all files match the filter under first [directory]/[path].
*
* You may pass multiple filter to it.
* Prefer this function over multiOpen, if there could be too many
* files that exceeds the systems file descriptor limit.
* @since 5.1.10
* @see multiOpen
*/
template <typename Arg1, typename... Args>
std::map<std::string, std::string>
locate(Type type, const std::string &path, Arg1 arg1, Args... args) const {
return locateWithFilter(type, path,
filter::Chainer<Arg1, Args...>(
std::move(arg1), std::move(args)...));
}

/// \brief Open all files match the first [directory]/[path].
std::vector<StandardPathFile> openAll(Type type, const std::string &path,
int flags) const;

/// \brief Open all files match the filter under first [directory]/[path].
StandardPathFileMap
multiOpenFilter(Type type, const std::string &path, int flags,
Expand Down
Loading

0 comments on commit a1e7502

Please sign in to comment.