[Rpm-maint] [rpm-software-management/rpm] rpmbuild: Fix `-ts` srpm specfile name (#1397)

[Michal Domonkos]

@dmnks commented on this pull request.



>      FD_t fd = NULL;
     static const char *tryspec[] = { "Specfile", "\\*.spec", NULL };
 
-    if (!(fd = rpmMkTempFile(NULL, &specFile)))
+    specDir = rpmGetPath("%{_tmppath}", NULL);

Cosmetic: For clarity, I would move this line to before the `specBase =` assignment further down, as it's only really used in that block.

> +    /* remove trailing \n */
+    specBase[strlen(specBase)-1] = '\0';
+
+    rasprintf(&specFile, "%s/%s", specDir, specBase);
+    res = rename(tmpSpecFile, specFile);
+
+    if (res) {
+       rpmlog(RPMLOG_ERR, _("Failed to rename %s to %s: %m\n"),
+               tmpSpecFile, specFile);
+       free(specFile);
+       specFile = NULL;
+    } else {
+       /* mkstemp() can give unnecessarily strict permissions, fixup */
+       mode_t mask;
+       umask(mask = umask(0));
+       (void) chmod(specFile, 0666 & ~mask);

It seems like this is no longer needed - with a plain `mkstemp(3)`, the file mode of the resulting file is undefined (as per the man page) and so needs to be manually set, however `rpmMkTempFile()` [takes care](https://github.com/rpm-software-management/rpm/blob/05fada7c9a1ef22bd310a9be4c6c06a4d2dd581d/rpmio/rpmfileutil.c#L58) of that for us.

>      FD_t fd = NULL;
     static const char *tryspec[] = { "Specfile", "\\*.spec", NULL };
 
-    if (!(fd = rpmMkTempFile(NULL, &specFile)))
+    specDir = rpmGetPath("%{_tmppath}", NULL);
+    if (!(fd = rpmMkTempFile(NULL, &tmpSpecFile)))

I'm thinking if it makes sense to create this as a temp file, only to be renamed to the real thing in the end? :)

Originally, I suppose, it was done because we didn't want to write the final name into `_specdir` before being done with it (as to not risk leaving an incomplete file behind in case of a failure), however now that we're keeping the file in `_tmpdir` even after the rename, we could perhaps just use the final name?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1397#pullrequestreview-541126603
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20201130/8128de33/attachment-0001.html>