From 792693bb90768cfde4898e8dd31ee1b5de803d2f Mon Sep 17 00:00:00 2001 From: Alexander Kanavin Date: Thu, 8 Jun 2017 17:08:09 +0300 Subject: [PATCH] build/pack.c: remove static local variables from buildHost() and getBuildTime() 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. Upstream-Status: Submitted [https://github.com/rpm-software-management/rpm/pull/226] Signed-off-by: Alexander Kanavin Signed-off-by: Alexander Kanavin --- build/build.c | 54 ++++++++++++++++++++++++++++-- build/pack.c | 84 +++++++++-------------------------------------- build/rpmbuild_internal.h | 8 +++-- 3 files changed, 74 insertions(+), 72 deletions(-) diff --git a/build/build.c b/build/build.c index 13c3df2..b154f08 100644 --- a/build/build.c +++ b/build/build.c @@ -6,6 +6,8 @@ #include "system.h" #include +#include +#include #include #include @@ -16,6 +18,50 @@ #include "debug.h" +static rpm_time_t getBuildTime(void) +{ + rpm_time_t buildTime = 0; + char *srcdate; + time_t epoch; + char *endptr; + + 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 char * buildHost(void) +{ + char* hostname; + struct hostent *hbn; + char *bhMacro; + + 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); +} + /** */ static rpmRC doRmSource(rpmSpec spec) @@ -201,6 +247,9 @@ static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec, int what) rpmRC rc = RPMRC_OK; int test = (what & RPMBUILD_NOBUILD); char *cookie = buildArgs->cookie ? xstrdup(buildArgs->cookie) : NULL; + const char* host = buildHost(); + rpm_time_t buildTime = getBuildTime(); + if (rpmExpandNumeric("%{?source_date_epoch_from_changelog}") && getenv("SOURCE_DATE_EPOCH") == NULL) { @@ -269,11 +318,11 @@ static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec, int what) goto exit; if (((what & RPMBUILD_PACKAGESOURCE) && !test) && - (rc = packageSources(spec, &cookie))) + (rc = packageSources(spec, &cookie, buildTime, host))) goto exit; if (((what & RPMBUILD_PACKAGEBINARY) && !test) && - (rc = packageBinaries(spec, cookie, (didBuild == 0)))) + (rc = packageBinaries(spec, cookie, (didBuild == 0), buildTime, host))) goto exit; if ((what & RPMBUILD_CLEAN) && @@ -293,6 +342,7 @@ static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec, int what) (void) unlink(spec->specFile); exit: + free(host); free(cookie); spec->rootDir = NULL; if (rc != RPMRC_OK && rpmlogGetNrecs() > 0) { diff --git a/build/pack.c b/build/pack.c index df15876..17a4b09 100644 --- a/build/pack.c +++ b/build/pack.c @@ -6,8 +6,6 @@ #include "system.h" #include -#include -#include #include #include /* RPMSIGTAG*, rpmReadPackageFile */ @@ -152,57 +150,6 @@ exit: return rc; } -static rpm_time_t * getBuildTime(void) -{ - static rpm_time_t buildTime[1]; - 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); - } - - return buildTime; -} - -static const char * buildHost(void) -{ - static char hostname[1024]; - static int oneshot = 0; - 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; - } - return(hostname); -} - static rpmRC processScriptFiles(rpmSpec spec, Package pkg) { struct TriggerFileEntry *p; @@ -476,7 +423,8 @@ exit: * order to how the RPM format is laid on disk. */ static rpmRC writeRPM(Package pkg, unsigned char ** pkgidp, - const char *fileName, char **cookie) + const char *fileName, char **cookie, + rpm_time_t buildTime, const char* buildHost) { FD_t fd = NULL; char * rpmio_flags = NULL; @@ -500,7 +448,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); } @@ -641,7 +589,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; @@ -660,8 +608,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); @@ -699,7 +647,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); @@ -719,7 +667,7 @@ struct binaryPackageTaskData struct binaryPackageTaskData *next; }; -static struct binaryPackageTaskData* runBinaryPackageTasks(rpmSpec spec, const char *cookie, int cheating) +static struct binaryPackageTaskData* runBinaryPackageTasks(rpmSpec spec, const char *cookie, int cheating, rpm_time_t buildTime, char* buildHost) { struct binaryPackageTaskData *tasks = NULL; struct binaryPackageTaskData *task = NULL; @@ -731,7 +679,7 @@ static struct binaryPackageTaskData* runBinaryPackageTasks(rpmSpec spec, const c 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, buildHost); rpmlog(RPMLOG_NOTICE, _("Finished binary package job, result %d, filename %s\n"), task->result, task->filename); tasks = task; } @@ -748,7 +696,7 @@ 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, buildHost); rpmlog(RPMLOG_NOTICE, _("Finished binary package job, result %d, filename %s\n"), task->result, task->filename); } } @@ -766,11 +714,11 @@ static void freeBinaryPackageTasks(struct binaryPackageTaskData* tasks) } } -rpmRC packageBinaries(rpmSpec spec, const char *cookie, int cheating) +rpmRC packageBinaries(rpmSpec spec, const char *cookie, int cheating, rpm_time_t buildTime, char* buildHost) { char *pkglist = NULL; - struct binaryPackageTaskData *tasks = runBinaryPackageTasks(spec, cookie, cheating); + struct binaryPackageTaskData *tasks = runBinaryPackageTasks(spec, cookie, cheating, buildTime, buildHost); for (struct binaryPackageTaskData *task = tasks; task != NULL; task = task->next) { if (task->result == RPMRC_OK) { @@ -797,7 +745,7 @@ rpmRC packageBinaries(rpmSpec spec, const char *cookie, int cheating) return RPMRC_OK; } -rpmRC packageSources(rpmSpec spec, char **cookie) +rpmRC packageSources(rpmSpec spec, char **cookie, rpm_time_t buildTime, char* buildHost) { Package sourcePkg = spec->sourcePackage; rpmRC rc; @@ -805,8 +753,8 @@ rpmRC packageSources(rpmSpec spec, char **cookie) /* Add some cruft */ headerPutString(sourcePkg->header, RPMTAG_RPMVERSION, VERSION); - headerPutString(sourcePkg->header, RPMTAG_BUILDHOST, buildHost()); - headerPutUint32(sourcePkg->header, RPMTAG_BUILDTIME, getBuildTime(), 1); + headerPutString(sourcePkg->header, RPMTAG_BUILDHOST, buildHost); + headerPutUint32(sourcePkg->header, RPMTAG_BUILDTIME, &buildTime, 1); headerPutUint32(sourcePkg->header, RPMTAG_SOURCEPACKAGE, &one, 1); /* XXX this should be %_srpmdir */ @@ -814,7 +762,7 @@ rpmRC packageSources(rpmSpec spec, char **cookie) 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, buildHost); /* Do check SRPM package if enabled */ if (rc == RPMRC_OK && pkgcheck[0] != ' ') { diff --git a/build/rpmbuild_internal.h b/build/rpmbuild_internal.h index 439b7d3..07e8338 100644 --- a/build/rpmbuild_internal.h +++ b/build/rpmbuild_internal.h @@ -427,19 +427,23 @@ rpmRC processSourceFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags); * @param spec spec file control structure * @param cookie build identifier "cookie" or NULL * @param cheating was build shortcircuited? + * @param buildTime the build timestamp that goes into packages + * @param buildHost the hostname where the build is happening * @return RPMRC_OK on success */ RPM_GNUC_INTERNAL -rpmRC packageBinaries(rpmSpec spec, const char *cookie, int cheating); +rpmRC packageBinaries(rpmSpec spec, const char *cookie, int cheating, rpm_time_t buildTime, char* buildHost); /** \ingroup rpmbuild * Generate source package. * @param spec spec file control structure * @retval cookie build identifier "cookie" or NULL + * @param buildTime the build timestamp that goes into packages + * @param buildHost the hostname where the build is happening * @return RPMRC_OK on success */ RPM_GNUC_INTERNAL -rpmRC packageSources(rpmSpec spec, char **cookie); +rpmRC packageSources(rpmSpec spec, char **cookie, rpm_time_t buildTime, char* buildHost); RPM_GNUC_INTERNAL int addLangTag(rpmSpec spec, Header h, rpmTagVal tag,