use fd for saving images

Closes: #244
Closes: #246

Author: N-R-K

TODO.md

@@ -61,9 +61,9 @@ transition for users and avoid depending on bleeding edge library versions.
 
 - [x] Require Imlib2 v1.11.0
 - [x] Document minimum version of other dependencies [#307](https://github.com/resurrecting-open-source-projects/scrot/issues/307)
-- [ ] Use file descriptors when saving images
-  - [ ] https://github.com/resurrecting-open-source-projects/scrot/issues/244
-  - [ ] https://github.com/resurrecting-open-source-projects/scrot/issues/246
+- [x] Use file descriptors when saving images
+  - [x] https://github.com/resurrecting-open-source-projects/scrot/issues/244
+  - [x] https://github.com/resurrecting-open-source-projects/scrot/issues/246
   - [ ] https://github.com/resurrecting-open-source-projects/scrot/issues/226
 - [x] Remove deprecated features
   - [x] --note  [#236](https://github.com/resurrecting-open-source-projects/scrot/issues/236)

src/options.c

@@ -361,18 +361,6 @@ static void optionsParseLine(char *optarg)
     } /* while */
 }
 
-static const char *getPathOfStdout(void)
-{
-    const char *paths[] = { "/dev/stdout", "/dev/fd/1", "/proc/self/fd/1" };
-
-    for (size_t i = 0; i < ARRAY_COUNT(paths); ++i) {
-        if (access(paths[i], W_OK) == 0)
-            return paths[i];
-    }
-    err(EXIT_FAILURE, "access to stdout failed");
-    return 0; /* silence tcc warning */
-}
-
 void optionsParse(int argc, char *argv[])
 {
     int optch;
@@ -526,7 +514,6 @@ void optionsParse(int argc, char *argv[])
     if (strcmp(opt.outputFile, "-") == 0) {
         opt.overwrite = true;
         opt.thumb = THUMB_DISABLED;
-        opt.outputFile = getPathOfStdout();
     }
 
     size_t outputFileLen = strlen(opt.outputFile);

src/scrot.c

@@ -48,8 +48,9 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #include <err.h>
 #include <errno.h>
-#include <stdio.h>
+#include <fcntl.h>
 #include <stdint.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <time.h>
@@ -68,15 +69,15 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 static void initXAndImlib(const char *, int);
 static void uninitXAndImlib(void);
-static void scrotSaveImage(const char *);
+static void scrotSaveImage(int, const char *);
 static Imlib_Image scrotGrabFocused(void);
 static Imlib_Image scrotGrabAutoselect(void);
 static long miliToNanoSec(int);
 static Imlib_Image scrotGrabShotMulti(void);
 static Imlib_Image scrotGrabShotMonitor(void);
 static Imlib_Image scrotGrabStackWindows(void);
 static Imlib_Image scrotGrabShot(void);
-static void scrotCheckIfOverwriteFile(char **);
+static int scrotCheckIfOverwriteFile(char **);
 static void scrotExecApp(Imlib_Image, struct tm *, char *, char *);
 static char *imPrintf(const char *, struct tm *, const char *, const char *,
     Imlib_Image);
@@ -102,6 +103,7 @@ int main(int argc, char *argv[])
     char *filenameThumb = NULL;
     struct timespec timeStamp;
     struct tm *tm;
+    int fd;
 
     /* Get the time ASAP to reduce the timing error in case --delay is used. */
     opt.delayStart = clockNow();
@@ -155,9 +157,8 @@ int main(int argc, char *argv[])
     imlib_image_attach_data_value("compression", NULL, opt.compression, NULL);
 
     filenameIM = imPrintf(opt.outputFile, tm, NULL, NULL, image);
-    scrotCheckIfOverwriteFile(&filenameIM);
-
-    scrotSaveImage(filenameIM);
+    fd = scrotCheckIfOverwriteFile(&filenameIM);
+    scrotSaveImage(fd, filenameIM);
 
     if (opt.thumb != THUMB_DISABLED) {
         int cwidth, cheight;
@@ -195,8 +196,8 @@ int main(int argc, char *argv[])
             imlib_image_set_format(opt.format);
 
             filenameThumb = imPrintf(opt.thumbFile, tm, NULL, NULL, thumbnail);
-            scrotCheckIfOverwriteFile(&filenameThumb);
-            scrotSaveImage(filenameThumb);
+            fd = scrotCheckIfOverwriteFile(&filenameThumb);
+            scrotSaveImage(fd, filenameThumb);
             imlib_free_image_and_decache();
         }
     }
@@ -254,9 +255,11 @@ static void uninitXAndImlib(void)
     }
 }
 
-static void scrotSaveImage(const char *filename)
+// save image to fd, filename only used for logging
+// fd will be closed after calling this function
+static void scrotSaveImage(int fd, const char *filename)
 {
-    imlib_save_image(filename);
+    imlib_save_image_fd(fd, filename);
     int imErr = imlib_get_error();
     if (imErr) {
         const char *errmsg = imlib_strerror(imErr);
@@ -537,43 +540,55 @@ static void scrotGrabMousePointer(Imlib_Image image, const int xOffset,
     XFree(xcim);
 }
 
-static void scrotCheckIfOverwriteFile(char **filename)
+static int scrotCheckIfOverwriteFile(char **filename)
 {
-    if (opt.overwrite)
-        return;
-
-    if (access(*filename, F_OK) == -1)
-        return;
-
-    const size_t maxCounter = 999;
-    char fmt[5]; // _000 + NUL byte
-    const size_t slen = strlen(*filename);
-    const size_t nalloc = slen + sizeof(fmt);
-
-    char *ext;
-    size_t extLength = scrotHaveFileExtension(*filename, &ext);
-
-    char *newName = ecalloc(nalloc, sizeof(*newName));
-    memcpy(newName, *filename, slen - extLength);
-    char *ptr = newName + (slen - extLength);
-
-    size_t counter = 0;
-    do {
-        snprintf(fmt, sizeof(fmt), "_%03zu", counter++);
-        memcpy(ptr, fmt, sizeof(fmt));
-        memcpy(ptr + sizeof(fmt) - 1, ext, extLength);
-    } while ((counter < maxCounter) && !access(newName, F_OK));
+    if (strcmp(*filename, "-") == 0) {
+        // scrotSaveImage will close the fd, so dup it
+        int fd = fcntl(1, F_DUPFD_CLOEXEC, 3);
+        if (fd < 0)
+            err(EXIT_FAILURE, "dup failed");
+        return fd;
+    }
 
-    scrotAssert(newName[nalloc - 1] == '\0');
+    int flags = O_RDWR | O_CREAT | (opt.overwrite ? O_TRUNC : O_EXCL);
+    int fd = open(*filename, flags, 0644);
+    if (!opt.overwrite && fd < 0 && errno == EEXIST) {
+        const size_t maxCounter = 999;
+        char fmt[5]; // _000 + NUL byte
+        const size_t slen = strlen(*filename);
+        const size_t nalloc = slen + sizeof(fmt);
+
+        char *ext;
+        size_t extLength = scrotHaveFileExtension(*filename, &ext);
+
+        char *newName = ecalloc(nalloc, sizeof(*newName));
+        memcpy(newName, *filename, slen - extLength);
+        char *ptr = newName + (slen - extLength);
+
+        size_t counter = 0;
+        do {
+            snprintf(fmt, sizeof(fmt), "_%03zu", counter++);
+            memcpy(ptr, fmt, sizeof(fmt));
+            memcpy(ptr + sizeof(fmt) - 1, ext, extLength);
+            fd = open(newName, flags, 0644);
+        } while ((counter < maxCounter) && fd < 0 && errno == EEXIST);
+        scrotAssert(newName[nalloc - 1] == '\0');
+
+        if (counter == maxCounter) {
+            errx(EXIT_FAILURE, "scrot can no longer generate new file names.\n"
+                "The last attempt is %s", newName);
+        }
 
-    if (counter == maxCounter) {
-        errx(EXIT_FAILURE, "scrot can no longer generate new file names.\n"
-            "The last attempt is %s", newName);
+        int saved_errno = errno; // avoid errno getting potentially clobbered
+        warnx("`%s` already exists, attempting `%s` instead", *filename, newName);
+        free(*filename);
+        *filename = newName;
+        errno = saved_errno;
     }
 
-    warnx("`%s` already exists, attempting `%s` instead", *filename, newName);
-    free(*filename);
-    *filename = newName;
+    if (fd < 0)
+        err(EXIT_FAILURE, "couldn't open file %s", *filename);
+    return fd;
 }
 
 static int scrotMatchWindowClassName(Window target)