Skip to content

Commit

Permalink
build/pack.c: remove static local variables from buildHost() and getB…
Browse files Browse the repository at this point in the history
…uildTime()

Their use is causing difficult to diagnoze data races when building multiple
packages in parallel, and is a bad idea in general, as it also makes it more
difficult to reason about code.
  • Loading branch information
kanavin committed Jun 9, 2017
1 parent 47189c9 commit c0fdce3
Showing 1 changed file with 43 additions and 44 deletions.
87 changes: 43 additions & 44 deletions build/pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -152,54 +152,47 @@ static rpmRC addFileToTag(rpmSpec spec, const char * file,
return rc;
}

static rpm_time_t * getBuildTime(void)
static rpm_time_t getBuildTime(void)
{
static rpm_time_t buildTime[1];
rpm_time_t buildTime = 0;
char *srcdate;
time_t epoch;
char *endptr;

if (buildTime[0] == 0) {
srcdate = getenv("SOURCE_DATE_EPOCH");
if (srcdate) {
errno = 0;
epoch = strtol(srcdate, &endptr, 10);
if (srcdate == endptr || *endptr || errno != 0)
rpmlog(RPMLOG_ERR, _("unable to parse SOURCE_DATE_EPOCH\n"));
else
buildTime[0] = (int32_t) epoch;
} else
buildTime[0] = (int32_t) time(NULL);
}
srcdate = getenv("SOURCE_DATE_EPOCH");
if (srcdate) {
errno = 0;
epoch = strtol(srcdate, &endptr, 10);
if (srcdate == endptr || *endptr || errno != 0)
rpmlog(RPMLOG_ERR, _("unable to parse SOURCE_DATE_EPOCH\n"));
else
buildTime = (int32_t) epoch;
} else
buildTime = (int32_t) time(NULL);

return buildTime;
}

static const char * buildHost(void)
static char * buildHost(void)
{
static char hostname[1024];
static int oneshot = 0;
char* hostname;
struct hostent *hbn;
char *bhMacro;

if (! oneshot) {
bhMacro = rpmExpand("%{?_buildhost}", NULL);
if (strcmp(bhMacro, "") != 0 && strlen(bhMacro) < 1024) {
strcpy(hostname, bhMacro);
} else {
if (strcmp(bhMacro, "") != 0)
rpmlog(RPMLOG_WARNING, _("The _buildhost macro is too long\n"));
(void) gethostname(hostname, sizeof(hostname));
hbn = gethostbyname(hostname);
if (hbn)
strcpy(hostname, hbn->h_name);
else
rpmlog(RPMLOG_WARNING,
_("Could not canonicalize hostname: %s\n"), hostname);
}
free(bhMacro);
oneshot = 1;
bhMacro = rpmExpand("%{?_buildhost}", NULL);
if (strcmp(bhMacro, "") != 0) {
rasprintf(&hostname, "%s", bhMacro);
} else {
hostname = rcalloc(1024, sizeof(*hostname));
(void) gethostname(hostname, 1024);
hbn = gethostbyname(hostname);
if (hbn)
strcpy(hostname, hbn->h_name);
else
rpmlog(RPMLOG_WARNING,
_("Could not canonicalize hostname: %s\n"), hostname);
}
free(bhMacro);
return(hostname);
}

Expand Down Expand Up @@ -494,7 +487,7 @@ static rpmRC writeRPM(Package pkg, unsigned char ** pkgidp,

/* Create and add the cookie */
if (cookie) {
rasprintf(cookie, "%s %d", buildHost(), (int) (*getBuildTime()));
rasprintf(cookie, "%s %d", buildHost, buildTime);
headerPutString(pkg->header, RPMTAG_COOKIE, *cookie);
}

Expand Down Expand Up @@ -633,7 +626,7 @@ static rpmRC checkPackages(char *pkgcheck)
return RPMRC_OK;
}

static rpmRC packageBinary(rpmSpec spec, Package pkg, const char *cookie, int cheating, char** filename)
static rpmRC packageBinary(rpmSpec spec, Package pkg, const char *cookie, int cheating, char** filename, rpm_time_t buildTime, const char* buildHost)
{
const char *errorString;
rpmRC rc = RPMRC_OK;
Expand All @@ -652,8 +645,8 @@ static rpmRC packageBinary(rpmSpec spec, Package pkg, const char *cookie, int ch
headerCopyTags(spec->packages->header, pkg->header, copyTags);

headerPutString(pkg->header, RPMTAG_RPMVERSION, VERSION);
headerPutString(pkg->header, RPMTAG_BUILDHOST, buildHost());
headerPutUint32(pkg->header, RPMTAG_BUILDTIME, getBuildTime(), 1);
headerPutString(pkg->header, RPMTAG_BUILDHOST, buildHost);
headerPutUint32(pkg->header, RPMTAG_BUILDTIME, &buildTime, 1);

if (spec->sourcePkgId != NULL) {
headerPutBin(pkg->header, RPMTAG_SOURCEPKGID, spec->sourcePkgId,16);
Expand Down Expand Up @@ -695,7 +688,7 @@ static rpmRC packageBinary(rpmSpec spec, Package pkg, const char *cookie, int ch
free(binRpm);
}

rc = writeRPM(pkg, NULL, *filename, NULL);
rc = writeRPM(pkg, NULL, *filename, NULL, buildTime, buildHost);
if (rc == RPMRC_OK) {
/* Do check each written package if enabled */
char *pkgcheck = rpmExpand("%{?_build_pkgcheck} ", *filename, NULL);
Expand All @@ -720,14 +713,16 @@ static struct binaryPackageTaskData* runBinaryPackageTasks(rpmSpec spec, const c
struct binaryPackageTaskData *tasks = NULL;
struct binaryPackageTaskData *task = NULL;
struct binaryPackageTaskData *prev = NULL;
rpm_time_t buildTime = getBuildTime();
char *host = buildHost();

for (Package pkg = spec->packages; pkg != NULL; pkg = pkg->next) {
task = rcalloc(1, sizeof(*task));
task->pkg = pkg;
if (pkg == spec->packages) {
// the first package needs to be processed ahead of others, as they copy
// changelog data from it, and so otherwise data races would happen
task->result = packageBinary(spec, pkg, cookie, cheating, &(task->filename));
task->result = packageBinary(spec, pkg, cookie, cheating, &(task->filename), buildTime, host);
rpmlog(RPMLOG_NOTICE, _("Finished binary package job, result %d, filename %s\n"), task->result, task->filename);
tasks = task;
}
Expand All @@ -743,11 +738,12 @@ static struct binaryPackageTaskData* runBinaryPackageTasks(rpmSpec spec, const c
if (task != tasks)
#pragma omp task
{
task->result = packageBinary(spec, task->pkg, cookie, cheating, &(task->filename));
task->result = packageBinary(spec, task->pkg, cookie, cheating, &(task->filename), buildTime, host);
rpmlog(RPMLOG_NOTICE, _("Finished binary package job, result %d, filename %s\n"), task->result, task->filename);
}
}

free(host);
return tasks;
}

Expand Down Expand Up @@ -798,16 +794,18 @@ rpmRC packageSources(rpmSpec spec, char **cookie)
rpmRC rc;

/* Add some cruft */
rpm_time_t buildTime = getBuildTime();
char* host = buildHost();
headerPutString(sourcePkg->header, RPMTAG_RPMVERSION, VERSION);
headerPutString(sourcePkg->header, RPMTAG_BUILDHOST, buildHost());
headerPutUint32(sourcePkg->header, RPMTAG_BUILDTIME, getBuildTime(), 1);
headerPutString(sourcePkg->header, RPMTAG_BUILDHOST, host);
headerPutUint32(sourcePkg->header, RPMTAG_BUILDTIME, &buildTime, 1);

/* XXX this should be %_srpmdir */
{ char *fn = rpmGetPath("%{_srcrpmdir}/", spec->sourceRpmName,NULL);
char *pkgcheck = rpmExpand("%{?_build_pkgcheck_srpm} ", fn, NULL);

spec->sourcePkgId = NULL;
rc = writeRPM(sourcePkg, &spec->sourcePkgId, fn, cookie);
rc = writeRPM(sourcePkg, &spec->sourcePkgId, fn, cookie, buildTime, host);

/* Do check SRPM package if enabled */
if (rc == RPMRC_OK && pkgcheck[0] != ' ') {
Expand All @@ -817,5 +815,6 @@ rpmRC packageSources(rpmSpec spec, char **cookie)
free(pkgcheck);
free(fn);
}
free(host);
return rc;
}

0 comments on commit c0fdce3

Please sign in to comment.