Skip to content

Commit

Permalink
cats: do not create a race condition with a member variable
Browse files Browse the repository at this point in the history
  • Loading branch information
franku authored and joergsteffens committed Sep 24, 2020
1 parent 8141b2b commit 9e83658
Show file tree
Hide file tree
Showing 13 changed files with 40 additions and 40 deletions.
2 changes: 1 addition & 1 deletion core/src/cats/bvfs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ void BareosDb::BvfsUpdateCache(JobControlRecord* jcr)
"ORDER BY JobId");
SqlQuery(cmd, DbListHandler, &jobids_list);

BvfsUpdatePathHierarchyCache(jcr, jobids_list.list());
BvfsUpdatePathHierarchyCache(jcr, jobids_list.GetAsString().c_str());

StartTransaction(jcr);
Dmsg0(dbglevel, "Cleaning pathvisibility\n");
Expand Down
10 changes: 1 addition & 9 deletions core/src/cats/cats.h
Original file line number Diff line number Diff line change
Expand Up @@ -498,15 +498,7 @@ class db_list_ctx : public BStringList {
void add(const char* str) { Append(str); }
void add(JobId_t id) { push_back(std::to_string(id)); }

std::string get_as_string() const { return Join(','); }
const char* list()
{
strlist_ = get_as_string();
return strlist_.c_str();
}

private:
std::string strlist_{};
std::string GetAsString() { return Join(','); }
};

typedef enum
Expand Down
2 changes: 1 addition & 1 deletion core/src/cats/sql_get.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1409,7 +1409,7 @@ bool BareosDb::AccurateGetJobids(JobControlRecord* jcr,
Mmsg(query, "SELECT JobId FROM btemp3%s ORDER by JobTDate", jobid);
}
SqlQueryWithHandler(query.c_str(), DbListHandler, jobids);
Dmsg1(1, "db_accurate_get_jobids=%s\n", jobids->list());
Dmsg1(1, "db_accurate_get_jobids=%s\n", jobids->GetAsString().c_str());
retval = true;

bail_out:
Expand Down
17 changes: 10 additions & 7 deletions core/src/dird/backup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ bool SendAccurateCurrentFiles(JobControlRecord* jcr)
*/
if (GetBaseJobids(jcr, &jobids)) {
jcr->HasBase = true;
Jmsg(jcr, M_INFO, 0, _("Using BaseJobId(s): %s\n"), jobids.list());
Jmsg(jcr, M_INFO, 0, _("Using BaseJobId(s): %s\n"),
jobids.GetAsString().c_str());
} else {
return true;
}
Expand Down Expand Up @@ -348,14 +349,16 @@ bool SendAccurateCurrentFiles(JobControlRecord* jcr)
/*
* To be able to allocate the right size for htable
*/
Mmsg(buf, "SELECT sum(JobFiles) FROM Job WHERE JobId IN (%s)", jobids.list());
Mmsg(buf, "SELECT sum(JobFiles) FROM Job WHERE JobId IN (%s)",
jobids.GetAsString().c_str());
jcr->db->SqlQuery(buf.c_str(), DbListHandler, &nb);
Dmsg2(200, "jobids=%s nb=%s\n", jobids.list(), nb.list());
jcr->file_bsock->fsend("accurate files=%s\n", nb.list());
Dmsg2(200, "jobids=%s nb=%s\n", jobids.GetAsString().c_str(),
nb.GetAsString().c_str());
jcr->file_bsock->fsend("accurate files=%s\n", nb.GetAsString().c_str());

if (jcr->HasBase) {
jcr->nb_base_files = str_to_int64(nb.list());
if (!jcr->db->CreateBaseFileList(jcr, jobids.list())) {
jcr->nb_base_files = str_to_int64(nb.GetAsString().c_str());
if (!jcr->db->CreateBaseFileList(jcr, jobids.GetAsString().c_str())) {
Jmsg(jcr, M_FATAL, 0, "error in jcr->db->CreateBaseFileList:%s\n",
jcr->db->strerror());
return false;
Expand All @@ -373,7 +376,7 @@ bool SendAccurateCurrentFiles(JobControlRecord* jcr)
}

jcr->db_batch->GetFileList(
jcr, jobids.list(), jcr->impl->use_accurate_chksum,
jcr, jobids.GetAsString().c_str(), jcr->impl->use_accurate_chksum,
false /* no delta */, AccurateListHandler, (void*)jcr);
}

Expand Down
10 changes: 6 additions & 4 deletions core/src/dird/consolidate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ bool DoConsolidate(JobControlRecord* jcr)
*/
jcr->db->AccurateGetJobids(jcr, &jcr->impl->jr, &jobids_ctx);
incrementals_total = jobids_ctx.size() - 1;
Dmsg1(10, "unlimited jobids list: %s.\n", jobids_ctx.list());
Dmsg1(10, "unlimited jobids list: %s.\n",
jobids_ctx.GetAsString().c_str());

/*
* If we are doing always incremental, we need to limit the search to
Expand All @@ -180,7 +181,8 @@ bool DoConsolidate(JobControlRecord* jcr)
}

jcr->db->AccurateGetJobids(jcr, &jcr->impl->jr, &jobids_ctx);
Dmsg1(10, "consolidate candidates: %s.\n", jobids_ctx.list());
Dmsg1(10, "consolidate candidates: %s.\n",
jobids_ctx.GetAsString().c_str());

/**
* Consolidation of zero or one job does not make sense, we leave it like
Expand Down Expand Up @@ -211,7 +213,7 @@ bool DoConsolidate(JobControlRecord* jcr)
jcr->db->AccurateGetJobids(jcr, &jcr->impl->jr, &jobids_ctx);
incrementals_to_consolidate = jobids_ctx.size() - 1;
Dmsg2(10, "%d consolidate ids after limit: %s.\n", jobids_ctx.size(),
jobids_ctx.list());
jobids_ctx.GetAsString().c_str());
if (incrementals_to_consolidate < 1) {
Jmsg(jcr, M_INFO, 0,
_("%s: After limited query: less incrementals than required, "
Expand All @@ -231,7 +233,7 @@ bool DoConsolidate(JobControlRecord* jcr)
jobids = NULL;
}

jobids = strdup(jobids_ctx.list());
jobids = strdup(jobids_ctx.GetAsString().c_str());
p = jobids;

/**
Expand Down
4 changes: 2 additions & 2 deletions core/src/dird/ua_cmds.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2391,14 +2391,14 @@ static bool DeleteVolume(UaContext* ua)
return true;
}
if (!lst.empty()) {
PurgeJobsFromCatalog(ua, lst.list());
PurgeJobsFromCatalog(ua, lst.GetAsString().c_str());
ua->send->ArrayStart("jobids");
for (const std::string& item : lst) { ua->send->ArrayItem(item.c_str()); }
ua->send->ArrayEnd("jobids");
ua->InfoMsg(
_("Deleted %d jobs and associated records deleted from the catalog "
"(jobids: %s).\n"),
lst.size(), lst.list());
lst.size(), lst.GetAsString().c_str());
}
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/dird/ua_dotcmds.cc
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ bool DotBvfsGetJobidsCmd(UaContext* ua, const char* cmd)
}
}

BvfsValidateJobids(ua, jobids.list(), filtered_jobids, false);
BvfsValidateJobids(ua, jobids.GetAsString().c_str(), filtered_jobids, false);
switch (ua->api) {
case API_MODE_JSON: {
char *cur_id, *bp;
Expand Down
9 changes: 5 additions & 4 deletions core/src/dird/ua_prune.cc
Original file line number Diff line number Diff line change
Expand Up @@ -771,22 +771,23 @@ static bool PruneBackupJobs(UaContext* ua,
* also remove BaseJobs that can be linked with them
*/
if (!jobids.empty()) {
Dmsg1(60, "jobids to exclude before basejobs = %s\n", jobids.list());
Dmsg1(60, "jobids to exclude before basejobs = %s\n",
jobids.GetAsString().c_str());
/* We also need to exclude all basejobs used */
ua->db->GetUsedBaseJobids(ua->jcr, jobids.list(), &jobids);
ua->db->GetUsedBaseJobids(ua->jcr, jobids.GetAsString().c_str(), &jobids);

/* Removing useful jobs from the DelCandidates list */
Mmsg(query,
"DELETE FROM DelCandidates "
"WHERE JobId IN (%s) " /* JobId used in accurate */
"AND JobFiles!=0", /* Discard when JobFiles=0 */
jobids.list());
jobids.GetAsString().c_str());

if (!ua->db->SqlQuery(query.c_str())) {
ua->ErrorMsg("%s", ua->db->strerror());
goto bail_out; /* Don't continue if the list isn't clean */
}
Dmsg1(60, "jobids to exclude = %s\n", jobids.list());
Dmsg1(60, "jobids to exclude = %s\n", jobids.GetAsString().c_str());
}

/* We use DISTINCT because we can have two times the same job */
Expand Down
2 changes: 1 addition & 1 deletion core/src/dird/ua_purge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ bool PurgeJobsFromVolume(UaContext* ua, MediaDbRecord* mr, bool force)
Dmsg0(050, "Count failed\n");
goto bail_out;
}
jobids = lst.get_as_string();
jobids = lst.GetAsString();
}

if (jobids.size() > 0) { PurgeJobsFromCatalog(ua, jobids.c_str()); }
Expand Down
6 changes: 3 additions & 3 deletions core/src/dird/ua_restore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,10 @@ static void GetAndDisplayBasejobs(UaContext* ua, RestoreContext* rx)

ua->SendMsg(_("The restore will use the following job(s) as Base\n"));
ua->db->FillQuery(query, BareosDb::SQL_QUERY::uar_print_jobs,
jobids.list());
jobids.GetAsString().c_str());
ua->db->ListSqlQuery(ua->jcr, query.c_str(), ua->send, HORZ_LIST, true);
}
PmStrcpy(rx->BaseJobIds, jobids.list());
PmStrcpy(rx->BaseJobIds, jobids.GetAsString().c_str());
}

static void free_rx(RestoreContext* rx)
Expand Down Expand Up @@ -772,7 +772,7 @@ static int UserSelectJobidsOrFiles(UaContext* ua, RestoreContext* rx)
jr.JobLevel = L_INCREMENTAL; /* Take Full+Diff+Incr */
if (!ua->db->AccurateGetJobids(ua->jcr, &jr, &jobids)) { return 0; }
}
PmStrcpy(rx->JobIds, jobids.list());
PmStrcpy(rx->JobIds, jobids.GetAsString().c_str());
Dmsg1(30, "Item 12: jobids = %s\n", rx->JobIds);
break;
case 12: /* Cancel or quit */
Expand Down
5 changes: 3 additions & 2 deletions core/src/dird/vbackup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,15 @@ bool DoNativeVbackup(JobControlRecord* jcr)
} else {
db_list_ctx jobids_ctx;
jcr->db->AccurateGetJobids(jcr, &jcr->impl->jr, &jobids_ctx);
Dmsg1(10, "consolidate candidates: %s.\n", jobids_ctx.list());
Dmsg1(10, "consolidate candidates: %s.\n",
jobids_ctx.GetAsString().c_str());

if (jobids_ctx.empty()) {
Jmsg(jcr, M_FATAL, 0, _("No previous Jobs found.\n"));
return false;
}

jobids = strdup(jobids_ctx.list());
jobids = strdup(jobids_ctx.GetAsString().c_str());
}

Jmsg(jcr, M_INFO, 0, _("Consolidating JobIds %s\n"), jobids);
Expand Down
6 changes: 3 additions & 3 deletions core/src/lib/output_formatter_resource.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#include "lib/util.h"
#include "lib/output_formatter_resource.h"

const char* GetAsString(void* item) { return (const char*)item; }
const char* GetAsCString(void* item) { return (const char*)item; }

OutputFormatterResource::OutputFormatterResource(OutputFormatter* send,
int indent_level)
Expand Down Expand Up @@ -229,7 +229,7 @@ void OutputFormatterResource::KeyMultipleStringsInOneLine(const char* key,
bool as_comment,
bool quoted_strings)
{
KeyMultipleStringsInOneLine(key, list, GetAsString, as_comment,
KeyMultipleStringsInOneLine(key, list, GetAsCString, as_comment,
quoted_strings);
}

Expand Down Expand Up @@ -288,7 +288,7 @@ void OutputFormatterResource::KeyMultipleStringsOnePerLine(const char* key,
bool as_comment,
bool quoted_strings)
{
KeyMultipleStringsOnePerLine(key, list, GetAsString, as_comment,
KeyMultipleStringsOnePerLine(key, list, GetAsCString, as_comment,
quoted_strings);
}

Expand Down
5 changes: 3 additions & 2 deletions core/src/tests/test_db_list_ctx.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ TEST(db_list_ctx, list)
list1.add(13);

EXPECT_EQ(3, list1.size());
EXPECT_EQ(0, list1.get_as_string().compare(std::string("11,12,13")));
EXPECT_EQ(0, list1.GetAsString().compare(std::string("11,12,13")));
// std::cerr << "list: " << list1.list() << "\n";
EXPECT_TRUE(bstrcmp(list1.list(), "11,12,13"));
EXPECT_TRUE(bstrcmp(list1.GetAsString().c_str(), "11,12,13"));
EXPECT_STREQ(list1.GetAsString().c_str(), "11,12,13");
}

0 comments on commit 9e83658

Please sign in to comment.