Skip to content

Commit

Permalink
fix problems listed by the linter...
Browse files Browse the repository at this point in the history
  • Loading branch information
HereThereBeDragons committed Dec 5, 2024
1 parent 19ebfba commit 5cdb8cf
Show file tree
Hide file tree
Showing 17 changed files with 103 additions and 85 deletions.
4 changes: 2 additions & 2 deletions cvmfs/catalog_mgr_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/

// avoid clang-tidy false positives (at least starting with clang14)
//NOLINTBEGIN
// NOLINTBEGIN

#ifndef CVMFS_CATALOG_MGR_IMPL_H_
#define CVMFS_CATALOG_MGR_IMPL_H_
Expand Down Expand Up @@ -1236,4 +1236,4 @@ void AbstractCatalogManager<CatalogT>::EnforceSqliteMemLimit() {


#endif // CVMFS_CATALOG_MGR_IMPL_H_
//NOLINTEND
// NOLINTEND
9 changes: 6 additions & 3 deletions cvmfs/cvmfs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,8 @@ static void cvmfs_lookup(fuse_req_t req, fuse_ino_t parent, const char *name) {
return;

lookup_reply_negative:
mount_point_->tracer()->Trace(Tracer::kEventLookup, path, "lookup()-NOTFOUND");
mount_point_->tracer()->Trace(Tracer::kEventLookup, path,
"lookup()-NOTFOUND");
// Will be a no-op if there is no fuse cache eviction
mount_point_->dentry_tracker()->Add(parent_fuse, name, uint64_t(timeout));
fuse_remounter_->fence()->Leave();
Expand All @@ -633,7 +634,8 @@ static void cvmfs_lookup(fuse_req_t req, fuse_ino_t parent, const char *name) {
return;

lookup_reply_error:
mount_point_->tracer()->Trace(Tracer::kEventLookup, path, "lookup()-NOTFOUND");
mount_point_->tracer()->Trace(Tracer::kEventLookup, path,
"lookup()-NOTFOUND");
fuse_remounter_->fence()->Leave();

LogCvmfs(kLogCvmfs, kLogDebug | kLogSyslogErr, "EIO (01) on %s", name);
Expand Down Expand Up @@ -1396,7 +1398,8 @@ static void cvmfs_read(fuse_req_t req, fuse_ino_t ino, size_t size, off_t off,

const struct fuse_ctx *fuse_ctx = fuse_req_ctx(req);
FuseInterruptCue ic(&req);
const ClientCtxGuard ctx_guard(fuse_ctx->uid, fuse_ctx->gid, fuse_ctx->pid, &ic);
const ClientCtxGuard ctx_guard(fuse_ctx->uid, fuse_ctx->gid, fuse_ctx->pid,
&ic);

// Do we have a a chunked file?
if (fd < 0) {
Expand Down
4 changes: 2 additions & 2 deletions cvmfs/mountpoint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1426,7 +1426,7 @@ bool MountPoint::CreateDownloadManagers() {
}

if (options_mgr_->GetValue("CVMFS_METALINK_URL", &optarg)) {
download_mgr_->SetMetalinkChain(optarg);
download_mgr_->SetMetalinkChain(optarg);
// host chain will be set later when the metalink server is contacted
download_mgr_->SetHostChain("");
// metalink requires redirects
Expand Down Expand Up @@ -2154,7 +2154,7 @@ bool MountPoint::SetupExternalDownloadMgr(bool dogeosort) {
external_download_mgr_->SetTimeout(timeout, timeout_direct);

if (options_mgr_->GetValue("CVMFS_EXTERNAL_METALINK", &optarg)) {
external_download_mgr_->SetMetalinkChain(optarg);
external_download_mgr_->SetMetalinkChain(optarg);
// host chain will be set later when the metalink server is contacted
external_download_mgr_->SetHostChain("");
// metalink requires redirects
Expand Down
59 changes: 28 additions & 31 deletions cvmfs/network/download.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1017,23 +1017,21 @@ void DownloadManager::InitializeRequest(JobInfo *info, CURL *handle) {

void DownloadManager::CheckHostInfoReset(
const std::string &typ,
HostInfo &info,
HostInfo *info,
JobInfo *jobinfo,
time_t &now)
{
if (info.timestamp_backup > 0) {
if (now == 0)
now = time(NULL);
if (static_cast<int64_t>(now) >
static_cast<int64_t>(info.timestamp_backup + info.reset_after))
{
time_t *now) {
if (info->timestamp_backup > 0) {
if (*now == 0)
*now = time(NULL);
if (static_cast<int64_t>(*now) >
static_cast<int64_t>(info->timestamp_backup + info->reset_after)) {
LogCvmfs(kLogDownload, kLogDebug | kLogSyslogWarn,
"(manager %s - id %" PRId64 ") "
"switching %s from %s to %s (reset %s)", name_.c_str(),
jobinfo->id(), typ.c_str(), (*info.chain)[info.current].c_str(),
(*info.chain)[0].c_str(), typ.c_str());
info.current = 0;
info.timestamp_backup = 0;
jobinfo->id(), typ.c_str(), (*info->chain)[info->current].c_str(),
(*info->chain)[0].c_str(), typ.c_str());
info->current = 0;
info->timestamp_backup = 0;
}
}
}
Expand Down Expand Up @@ -1113,8 +1111,8 @@ void DownloadManager::SetUrlOptions(JobInfo *info) {
} // end !sharding

// Check if metalink and host chains need to be reset
CheckHostInfoReset("metalink", opt_metalink_, info, now);
CheckHostInfoReset("host", opt_metalink_, info, now);
CheckHostInfoReset("metalink", &opt_metalink_, info, &now);
CheckHostInfoReset("host", &opt_metalink_, info, &now);

curl_easy_setopt(curl_handle, CURLOPT_LOW_SPEED_LIMIT, opt_low_speed_limit_);
if (info->proxy() != "DIRECT") {
Expand Down Expand Up @@ -1421,7 +1419,6 @@ static bool sortlinks(const std::string &s1, const std::string &s2) {
* See rfc6249.
*/
void DownloadManager::ProcessLink(JobInfo *info) {

std::vector<std::string> links = SplitString(info->link(), ',');
if (info->link().find("; pri=") != std::string::npos)
std::sort(links.begin(), links.end(), sortlinks);
Expand Down Expand Up @@ -2379,11 +2376,11 @@ void DownloadManager::SwitchProxy(JobInfo *info) {
* transfer has already done the switch.
*/
void DownloadManager::SwitchHostInfo(const std::string &typ,
HostInfo &info,
HostInfo *info,
JobInfo *jobinfo) {
MutexLockGuard m(lock_options_);

if (!info.chain || (info.chain->size() == 1)) {
if (!info->chain || (info->chain->size() == 1)) {
return;
}

Expand All @@ -2394,14 +2391,14 @@ void DownloadManager::SwitchHostInfo(const std::string &typ,
} else {
lastused = jobinfo->current_metalink_chain_index();
}
if (lastused != info.current) {
if (lastused != info->current) {
LogCvmfs(kLogDownload, kLogDebug,
"(manager '%s' - id %" PRId64 ")"
"don't switch %s, "
"last used %s: %s, current %s: %s",
name_.c_str(), jobinfo->id(), typ.c_str(),
typ.c_str(), (*info.chain)[lastused].c_str(),
typ.c_str(), (*info.chain)[info.current].c_str());
typ.c_str(), (*info->chain)[lastused].c_str(),
typ.c_str(), (*info->chain)[info->current].c_str());
return;
}
}
Expand All @@ -2414,31 +2411,31 @@ void DownloadManager::SwitchHostInfo(const std::string &typ,
}
info_id += ")";

const std::string old_host = (*info.chain)[info.current];
info.current = (info.current + 1) % static_cast<int>(info.chain->size());
const std::string old_host = (*info->chain)[info->current];
info->current = (info->current + 1) % static_cast<int>(info->chain->size());
if (typ == "host") {
perf::Inc(counters_->n_host_failover);
} else {
perf::Inc(counters_->n_metalink_failover);
}
LogCvmfs(kLogDownload, kLogDebug | kLogSyslogWarn,
"%s switching %s from %s to %s (%s)", info_id.c_str(), typ.c_str(),
old_host.c_str(), (*info.chain)[info.current].c_str(),
old_host.c_str(), (*info->chain)[info->current].c_str(),
reason.c_str());

// Remember the timestamp of switching to backup host
if (info.reset_after > 0) {
if (info.current != 0) {
if (info.timestamp_backup == 0)
info.timestamp_backup = time(NULL);
if (info->reset_after > 0) {
if (info->current != 0) {
if (info->timestamp_backup == 0)
info->timestamp_backup = time(NULL);
} else {
info.timestamp_backup = 0;
info->timestamp_backup = 0;
}
}
}

void DownloadManager::SwitchHost(JobInfo *info) {
SwitchHostInfo("host", opt_host_, info);
SwitchHostInfo("host", &opt_host_, info);
}

void DownloadManager::SwitchHost() {
Expand All @@ -2447,7 +2444,7 @@ void DownloadManager::SwitchHost() {


void DownloadManager::SwitchMetalink(JobInfo *info) {
SwitchHostInfo("metalink", opt_metalink_, info);
SwitchHostInfo("metalink", &opt_metalink_, info);
}


Expand Down
6 changes: 3 additions & 3 deletions cvmfs/network/download.h
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ class DownloadManager { // NOLINT(clang-analyzer-optin.performance.Padding)
bool ValidateGeoReply(const std::string &reply_order,
const unsigned expected_size,
std::vector<uint64_t> *reply_vals);
void SwitchHostInfo(const std::string &typ, HostInfo &info, JobInfo *jobinfo);
void SwitchHostInfo(const std::string &typ, HostInfo *info, JobInfo *jobinfo);
void SwitchMetalink(JobInfo *info);
void SwitchHost(JobInfo *info);
void SwitchProxy(JobInfo *info);
Expand All @@ -292,8 +292,8 @@ class DownloadManager { // NOLINT(clang-analyzer-optin.performance.Padding)
bool VerifyAndFinalize(const int curl_error, JobInfo *info);
void InitHeaders();
void CloneProxyConfig(DownloadManager *clone);
void CheckHostInfoReset(const std::string &typ, HostInfo &info,
JobInfo *jobinfo, time_t &now);
void CheckHostInfoReset(const std::string &typ, HostInfo *info,
JobInfo *jobinfo, time_t *now);

bool EscapeUrlChar(unsigned char input, char output[3]);
std::string EscapeUrl(const int64_t jobinfo_id, const std::string &url);
Expand Down
46 changes: 29 additions & 17 deletions cvmfs/quota_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -292,14 +292,19 @@ PosixQuotaManager *PosixQuotaManager::CreateShared(
LogCvmfs(kLogQuota, kLogDebug, "trying to connect to existing pipe");
quota_mgr->pipe_lru_[1] = open(fifo_path.c_str(), O_WRONLY | O_NONBLOCK);
if (quota_mgr->pipe_lru_[1] >= 0) {
const int fd_lockfile_rw = open((workspace_dir + "/lock_cachemgr").c_str(), O_RDWR, 0600);
const int fd_lockfile_rw = open((workspace_dir + "/lock_cachemgr").c_str(),
O_RDWR, 0600);
unsigned lockfile_magicnumber = 0;
const ssize_t result_mn = SafeRead(fd_lockfile_rw, &lockfile_magicnumber, sizeof(lockfile_magicnumber));
const ssize_t result = SafeRead(fd_lockfile_rw, &new_cachemgr_pid, sizeof(new_cachemgr_pid));
const ssize_t result_mn = SafeRead(fd_lockfile_rw, &lockfile_magicnumber,
sizeof(lockfile_magicnumber));
const ssize_t result = SafeRead(fd_lockfile_rw, &new_cachemgr_pid,
sizeof(new_cachemgr_pid));
close(fd_lockfile_rw);

if ((lockfile_magicnumber != kLockFileMagicNumber) || (result < 0) || (result_mn < 0)
|| (static_cast<size_t>(result) < sizeof(new_cachemgr_pid))) {
if ((lockfile_magicnumber != kLockFileMagicNumber)
|| (result < 0)
|| (result_mn < 0)
|| (static_cast<size_t>(result) < sizeof(new_cachemgr_pid))) {
if (result != 0) {
LogCvmfs(kLogQuota, kLogDebug | kLogSyslogErr,
"could not read cache manager pid from lockfile");
Expand Down Expand Up @@ -377,7 +382,7 @@ PosixQuotaManager *PosixQuotaManager::CreateShared(
command_line.push_back(StringifyInt(cleanup_threshold));
// do not propagate foreground in order to reliably get pid from exec
// instead, daemonize right here
command_line.push_back(StringifyInt(true)); //foreground
command_line.push_back(StringifyInt(true)); // foreground
command_line.push_back(StringifyInt(GetLogSyslogLevel()));
command_line.push_back(StringifyInt(GetLogSyslogFacility()));
command_line.push_back(GetLogDebugFile() + ":" + GetLogMicroSyslog());
Expand Down Expand Up @@ -408,10 +413,13 @@ PosixQuotaManager *PosixQuotaManager::CreateShared(
}
LogCvmfs(kLogQuota, kLogDebug, "new cache manager pid: %d", new_cachemgr_pid);
quota_mgr->SetCacheMgrPid(new_cachemgr_pid);
const int fd_lockfile_rw = open((workspace_dir + "/lock_cachemgr").c_str(), O_RDWR | O_TRUNC, 0600);
const int fd_lockfile_rw = open((workspace_dir + "/lock_cachemgr").c_str(),
O_RDWR | O_TRUNC, 0600);
const unsigned magic_number = PosixQuotaManager::kLockFileMagicNumber;
const bool result_mn = SafeWrite(fd_lockfile_rw, &magic_number, sizeof(magic_number));
const bool result = SafeWrite(fd_lockfile_rw, &new_cachemgr_pid, sizeof(new_cachemgr_pid));
const bool result_mn = SafeWrite(fd_lockfile_rw, &magic_number,
sizeof(magic_number));
const bool result = SafeWrite(fd_lockfile_rw, &new_cachemgr_pid,
sizeof(new_cachemgr_pid));
if (!result || !result_mn) {
PANIC(kLogSyslogErr, "could not write cache manager pid to lockfile");
}
Expand Down Expand Up @@ -764,11 +772,13 @@ bool PosixQuotaManager::SetSharedLimit(uint64_t limit) {
}


bool PosixQuotaManager::SetLimit( uint64_t size) {
bool PosixQuotaManager::SetLimit(uint64_t size) {
if (!spawned_) {
limit_ = size;
cleanup_threshold_ = size/2;
LogCvmfs(kLogQuota, kLogDebug | kLogSyslog, "Quota limit set to %lu / threshold %lu", limit_, cleanup_threshold_ );
LogCvmfs(kLogQuota, kLogDebug | kLogSyslog,
"Quota limit set to %lu / threshold %lu",
limit_, cleanup_threshold_);
return true;
}
return SetSharedLimit(size);
Expand Down Expand Up @@ -801,7 +811,8 @@ uint64_t PosixQuotaManager::GetCleanupRate(uint64_t period_s) {
cmd.size = period_s;
cmd.return_pipe = pipe_cleanup_rate[1];
WritePipe(pipe_lru_[1], &cmd, sizeof(cmd));
ManagedReadHalfPipe(pipe_cleanup_rate[0], &cleanup_rate, sizeof(cleanup_rate));
ManagedReadHalfPipe(pipe_cleanup_rate[0], &cleanup_rate,
sizeof(cleanup_rate));
CloseReturnPipe(pipe_cleanup_rate);

return cleanup_rate;
Expand Down Expand Up @@ -1057,7 +1068,6 @@ vector<string> PosixQuotaManager::ListVolatile() {
* Entry point for the shared cache manager process
*/
int PosixQuotaManager::MainCacheManager(int argc, char **argv) {

LogCvmfs(kLogQuota, kLogDebug, "starting quota manager");
int retval;

Expand Down Expand Up @@ -1247,9 +1257,11 @@ void *PosixQuotaManager::MainCommandServer(void *data) {
quota_mgr->BindReturnPipe(command_buffer[num_commands].return_pipe);
if (return_pipe < 0)
continue;
quota_mgr->limit_ = size; // use the size field to transmit the size
quota_mgr->limit_ = size; // use the size field to transmit the size
quota_mgr->cleanup_threshold_ = size/2;
LogCvmfs(kLogQuota, kLogDebug | kLogSyslog, "Quota limit set to %lu / threshold %lu", quota_mgr->limit_, quota_mgr->cleanup_threshold_ );
LogCvmfs(kLogQuota, kLogDebug | kLogSyslog,
"Quota limit set to %lu / threshold %lu",
quota_mgr->limit_, quota_mgr->cleanup_threshold_);
bool ret = true;
WritePipe(return_pipe, &ret, sizeof(ret));
quota_mgr->UnbindReturnPipe(return_pipe);
Expand Down Expand Up @@ -2057,7 +2069,7 @@ void PosixQuotaManager::ManagedReadHalfPipe(int fd, void *buf, size_t nbyte) {
// try only as long as the cachemgr is still alive
} while (!result && getpgid(cachemgr_pid_) >= 0);
if (!result) {
PANIC(kLogStderr, "Error: quota manager could not read from cachemanager pipe");
PANIC(kLogStderr,
"Error: quota manager could not read from cachemanager pipe");
}

}
2 changes: 1 addition & 1 deletion cvmfs/quota_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class PosixQuotaManager : public QuotaManager {
virtual uint32_t GetProtocolRevision();

void ManagedReadHalfPipe(int fd, void *buf, size_t nbyte);
void SetCacheMgrPid(pid_t pid_) { cachemgr_pid_ = pid_;};
void SetCacheMgrPid(pid_t pid_) { cachemgr_pid_ = pid_; }


private:
Expand Down
4 changes: 2 additions & 2 deletions cvmfs/receiver/catalog_merge_tool.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ class CatalogMergeTool : public CatalogDiffTool<RoCatalogMgr> {

virtual ~CatalogMergeTool() {}

bool Run(const Params& params, std::string* new_manifest_path, shash::Any* new_manifest_hash,
uint64_t *final_rev);
bool Run(const Params& params, std::string* new_manifest_path,
shash::Any* new_manifest_hash, uint64_t *final_rev);

protected:
virtual bool IsIgnoredPath(const PathString& path);
Expand Down
3 changes: 2 additions & 1 deletion cvmfs/receiver/catalog_merge_tool_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ namespace receiver {

template <typename RwCatalogMgr, typename RoCatalogMgr>
bool CatalogMergeTool<RwCatalogMgr, RoCatalogMgr>::Run(
const Params& params, std::string* new_manifest_path, shash::Any* new_manifest_hash, uint64_t *final_rev) {
const Params& params, std::string* new_manifest_path,
shash::Any* new_manifest_hash, uint64_t *final_rev) {
UniquePtr<upload::Spooler> spooler;
perf::StatisticsTemplate stats_tmpl("publish", statistics_);
counters_ = new perf::FsCounters(stats_tmpl);
Expand Down
3 changes: 2 additions & 1 deletion cvmfs/receiver/commit_processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ CommitProcessor::Result CommitProcessor::Process(

std::string new_manifest_path;
shash::Any new_manifest_hash;
if (!merge_tool.Run(params, &new_manifest_path, &new_manifest_hash, final_revision)) {
if (!merge_tool.Run(params, &new_manifest_path, &new_manifest_hash,
final_revision)) {
LogCvmfs(kLogReceiver, kLogSyslogErr,
"CommitProcessor - error: Catalog merge failed");
return kMergeFailure;
Expand Down
2 changes: 1 addition & 1 deletion cvmfs/ring_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,4 @@ class RingBuffer : SingleCopy {
unsigned char *buffer_;
}; // class RingBuffer

#endif // CVMFS_RING_H_
#endif // CVMFS_RING_BUFFER_H_
1 change: 0 additions & 1 deletion cvmfs/swissknife_check.cc
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,6 @@ catalog::Catalog* CommandCheck::FetchCatalog(const string &path,
catalog::Catalog::AttachFreely(path, tmp_file, catalog_hash);
int64_t catalog_file_size = GetFileSize(tmp_file);
if (catalog_file_size <= 0) {

LogCvmfs(kLogCvmfs, kLogStderr, "Error downloading catalog %s at %s %s",
catalog_hash.ToString().c_str(), path.c_str(), tmp_file.c_str() );
assert(catalog_file_size > 0);
Expand Down
2 changes: 1 addition & 1 deletion cvmfs/swissknife_migrate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,7 @@ bool CommandMigrate::MigrationWorker_20x::MigrateFileMetadata(
" IFNULL(hardlink_group_id, 0) << 32 | "
" COALESCE(hardlinks.linkcount, dir_linkcounts.linkcount, 1) "
" AS hardlinks, "
" hash, size, mode, mtime, NULL, " // set empty mtimens
" hash, size, mode, mtime, NULL, " // set empty mtimens
" flags, name, symlink, "
" :uid, "
" :gid, "
Expand Down
Loading

0 comments on commit 5cdb8cf

Please sign in to comment.