From ec305795a302d226343e69031ff2024dfcde69c0 Mon Sep 17 00:00:00 2001 From: Alexander Kanavin Date: Thu, 8 Jun 2017 17:08:09 +0300 Subject: [PATCH 3/3] 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 5f99c8db7..09a1311c5 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) @@ -203,6 +249,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) { @@ -271,11 +320,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))) return rc; 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) && @@ -295,6 +344,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 ed5b9ab4e..62427065a 100644 --- a/build/pack.c +++ b/build/pack.c @@ -6,8 +6,6 @@ #include "system.h" #include -#include -#include #include #include /* RPMSIGTAG*, rpmReadPackageFile */ @@ -151,57 +149,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; @@ -308,7 +255,8 @@ static int haveRichDep(Package pkg) } 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; @@ -397,7 +345,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); } @@ -546,7 +494,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; @@ -565,8 +513,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); @@ -604,7 +552,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); @@ -624,7 +572,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; @@ -636,7 +584,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; } @@ -653,7 +601,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); } } @@ -671,11 +619,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) { @@ -702,22 +650,22 @@ 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; /* 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); /* 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, 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 8351a6a34..797337ca7 100644 --- a/build/rpmbuild_internal.h +++ b/build/rpmbuild_internal.h @@ -408,19 +408,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, -- 2.11.0