m + split out save_fbs to main + do not use PATH_MAX + do not use alloca

Signed-off-by: Mattias Andrée <maandree@member.fsf.org>

6 files changed, 170 insertions, 81 deletions

diff --git a/src/common.h b/src/common.h
index 93753b6..573a257 100644
--- a/src/common.h
+++ b/src/common.h
@@ -16,7 +16,12 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
-#define _POSIX_SOURCE
+#if !defined(_POSIX_C_SOURCE) || (_POSIX_C_SOURCE < 200809L)
+# if defined(_POSIX_C_SOURCE)
+#  undef _POSIX_C_SOURCE
+# endif
+# define _POSIX_C_SOURCE  200809L
+#endif
 #include <stdio.h>
 #include <fcntl.h>
 #include <unistd.h>
@@ -24,7 +29,6 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
-#include <alloca.h>
 #include <time.h>
 
 
@@ -42,9 +46,6 @@
 #ifndef SYSDIR
 # define SYSDIR  "/sys"
 #endif
-#ifndef PATH_MAX  /* TODO can we avoid this and still have clean code */
-# define PATH_MAX  4096
-#endif
 
 
 /**
diff --git a/src/kern-linux.c b/src/kern-linux.c
index a3d4ea2..fdae636 100644
--- a/src/kern-linux.c
+++ b/src/kern-linux.c
@@ -49,14 +49,17 @@ print_not_found_help (void)
 /**
  * Construct the path to a framebuffer device.
  * 
- * @param  pathbuf  Ouput buffer for the path.
- * @param  altpath  The index of the alternative path-pattern to use.
- * @param  fbno     The index of the framebuffer.
+ * @param   altpath  The index of the alternative path-pattern to use.
+ * @param   fbno     The index of the framebuffer.
+ * @return           The path to the framebuffer device. Errors are impossible.
+ *                   This string is statically allocated and must not be deallocated.
  */
-void
-get_fbpath (char *restrict pathbuf, int altpath, int fbno)
+char *
+get_fbpath (int altpath, int fbno)
 {
+  static char pathbuf[sizeof (DEVDIR "/fb/") + 3 * sizeof (int)];
   sprintf (pathbuf, "%s/fb%s%i", DEVDIR, (altpath ? "/" : ""), fbno);
+  return pathbuf;
 }
 
 
@@ -72,7 +75,8 @@ get_fbpath (char *restrict pathbuf, int altpath, int fbno)
 int
 measure (int fbno, char *restrict fbpath, long *restrict width, long *restrict height)
 {
-  static char buf[PATH_MAX];
+  static char buf[sizeof (SYSDIR "/class/graphics/fb/virtual_size") + 3 * sizeof(int)];
+  /* The string "/class/graphics/fb/virtual_size" is large enought for the call (*) "*/
   char *delim;
   int sizefd = -1;
   ssize_t got;
@@ -85,7 +89,7 @@ measure (int fbno, char *restrict fbpath, long *restrict width, long *restrict h
     FILE_FAILURE (buf);
   
   /* Get the dimensions of the framebuffer. */
-  got = read (sizefd, buf, sizeof (buf) / sizeof (char) - 1);
+  got = read (sizefd, buf, sizeof (buf) / sizeof (char) - 1); /* (*) */
   if (got < 0)
     goto fail;
   close (sizefd);
diff --git a/src/kern.h b/src/kern.h
index 624564c..45f692a 100644
--- a/src/kern.h
+++ b/src/kern.h
@@ -35,11 +35,12 @@ void print_not_found_help (void);
 /**
  * Construct the path to a framebuffer device.
  * 
- * @param  pathbuf  Ouput buffer for the path.
- * @param  altpath  The index of the alternative path-pattern to use.
- * @param  fbno     The index of the framebuffer.
+ * @param   altpath  The index of the alternative path-pattern to use.
+ * @param   fbno     The index of the framebuffer.
+ * @return           The path to the framebuffer device. Errors are impossible.
+ *                   This string is statically allocated and must not be deallocated.
  */
-void get_fbpath (char *restrict pathbuf, int altpath, int fbno);
+char *get_fbpath (int altpath, int fbno);
 
 /**
  * Get the dimensions of a framebuffer.
diff --git a/src/pattern.c b/src/pattern.c
index c766d83..8031469 100644
--- a/src/pattern.c
+++ b/src/pattern.c
@@ -71,9 +71,9 @@ duplicate_percents (char *restrict buf, size_t n)
  *                   during the evaluation of the filename pattern.
  * @return           Zero on success, -1 on error.
  */
-int
-evaluate (char *restrict buf, size_t n, const char *restrict pattern,
-	  int fbno, long width, long height, const char *restrict path)
+static int
+try_evaluate (char *restrict buf, size_t n, const char *restrict pattern,
+	      int fbno, long width, long height, const char *restrict path)
 {
 #define P(format, value)  r = snprintf (buf + i, n - i, format "%zn", value, &j)
   
@@ -81,9 +81,10 @@ evaluate (char *restrict buf, size_t n, const char *restrict pattern,
   ssize_t j = 0;
   int percent = 0, backslash = 0, dollar = 0, r;
   char c;
-  char *fmt;
+  char *fmt = NULL;
   time_t t;
   struct tm *tm;
+  int saved_errno;
   
   /* Expand '$' and '\'. */
   while ((i < n) && ((c = *pattern++)))
@@ -123,8 +124,9 @@ evaluate (char *restrict buf, size_t n, const char *restrict pattern,
     return 0;
   
   /* Copy the buffer so we can reuse the buffer and use its old content for the format. */
-  fmt = alloca ((strlen (buf) + 1) * sizeof (char));
-  memcpy (fmt, buf, (strlen (buf) + 1) * sizeof (char));
+  fmt = strdup (buf);
+  if (fmt == NULL)
+    goto fail;
   
   /* Expand '%'. */
   t = time (NULL);
@@ -138,12 +140,62 @@ evaluate (char *restrict buf, size_t n, const char *restrict pattern,
   if (strftime (buf, n, fmt, tm) == 0)
     goto enametoolong; /* No errors are defined for `strftime`. What else can we do? */
   
+  free (fmt);
   return 0;
   
  enametoolong:
-  return errno = ENAMETOOLONG, -1;
-  
+  errno = ENAMETOOLONG;
  fail:
+  saved_errno = errno;
+  free (fmt);
+  errno = saved_errno;
   return -1;
 }
 
+
+/**
+ * Parse and evaluate a --exec argument or filename pattern.
+ * 
+ * If `path != NULL` than all non-escaped spaces in
+ * `pattern` will be stored as 255-bytes in `buf`.
+ * 
+ * @param   pattern  The pattern to evaluate.
+ * @param   fbno     The index of the framebuffer.
+ * @param   width    The width of the image/framebuffer.
+ * @param   height   The height of the image/framebuffer.
+ * @param   path     The filename of the saved image, `NULL`
+ *                   during the evaluation of the filename pattern.
+ * @return           The constructed string, `NULL` on error.
+ */
+char*
+evaluate (const char *restrict pattern, int fbno, long width,
+	  long height, const char *restrict path)
+{
+  char *buffer = NULL;
+  size_t size = 32;
+  void *new;
+  int saved_errno;
+  
+ retry:
+  new = realloc (buffer, size * sizeof(char));
+  if (new == NULL)
+    goto fail;
+  buffer = new;
+  
+  if (try_evaluate (buffer, size, pattern, fbno, width, height, path) < 0)
+    {
+      if (errno == ENAMETOOLONG)
+	goto retry;
+      size <<= 1;
+      goto fail;
+    }
+  
+  return buffer;
+  
+ fail:
+  saved_errno = errno;
+  free(buffer);
+  errno = saved_errno;
+  return NULL;
+}
+
diff --git a/src/pattern.h b/src/pattern.h
index 98bc7a3..a3eef59 100644
--- a/src/pattern.h
+++ b/src/pattern.h
@@ -25,16 +25,14 @@
  * If `path != NULL` than all non-escaped spaces in
  * `pattern` will be stored as 255-bytes in `buf`.
  * 
- * @param   buf      The output buffer.
- * @param   n        The size of `buf`.
  * @param   pattern  The pattern to evaluate.
  * @param   fbno     The index of the framebuffer.
  * @param   width    The width of the image/framebuffer.
  * @param   height   The height of the image/framebuffer.
  * @param   path     The filename of the saved image, `NULL`
  *                   during the evaluation of the filename pattern.
- * @return           Zero on success, -1 on error.
+ * @return           The constructed string, `NULL` on error.
  */
-int evaluate (char *restrict buf, size_t n, const char *restrict pattern,
-	      int fbno, long width, long height, const char *restrict path);
+char *evaluate (const char *restrict pattern, int fbno, long width,
+		long height, const char *restrict path);
 
diff --git a/src/scrotty.c b/src/scrotty.c
index 9105368..5c7379d 100644
--- a/src/scrotty.c
+++ b/src/scrotty.c
@@ -101,7 +101,7 @@ static int try_alt_fbpath = 0;
 static int
 save_pnm (const char *fbpath, long width, long height, int fd)
 {
-  char buf[PATH_MAX];
+  char buf[8 << 10];
   FILE *file = NULL;
   int fbfd = 1;
   ssize_t got, off;
@@ -280,11 +280,11 @@ save (const char *fbpath, const char *imgpath, long width, long height, int raw)
 static int
 exec_image (char *flatten_args)
 {
-  char **args;
+  char **args = NULL;
   char *arg;
   size_t i, arg_count = 1;
   pid_t pid;
-  int status;
+  int status, saved_errno;
   
   /* Count arguments. */
   for (i = 0; flatten_args[i]; i++)
@@ -292,7 +292,9 @@ exec_image (char *flatten_args)
       arg_count++;
   
   /* Allocate argument array. */
-  args = alloca ((arg_count + 1) * sizeof (char*));
+  args = malloc ((arg_count + 1) * sizeof (char*));
+  if (args == NULL)
+    goto fail;
   
   /* Unflatten argument array. */
   for (arg = flatten_args, i = 0;;)
@@ -308,7 +310,7 @@ exec_image (char *flatten_args)
   /* Fork process. */
   pid = fork ();
   if (pid == -1)
-    return -1;
+    goto fail;
 
   /* Child process: */
   if (pid == 0)
@@ -322,10 +324,17 @@ exec_image (char *flatten_args)
   
   /* Wait for child to exit. */
   if (waitpid (pid, &status, 0) < 0)
-    return -1;
+    goto fail;
   
   /* Return successfully if and only if `the child` did. */
+  free (args);
   return status == 0 ? 0 : -1;
+  
+ fail:
+  saved_errno = errno;
+  free (args);
+  errno = saved_errno;
+  return -1;
 }
 
 
@@ -336,22 +345,21 @@ exec_image (char *flatten_args)
  * @param   raw          Save in PNM rather than in PNG?.
  * @param   filepattern  The pattern for the filename, `NULL` for default.
  * @param   execpattern  The pattern for the command to run to
- *                       process the image, `NULL` for default.
+ *                       process the image, `NULL` for none.
  * @return               Zero on success, -1 on error, 1 if the framebuffer does not exist.
  */
 static int
 save_fb (int fbno, int raw, const char *filepattern, const char *execpattern)
 {
-  static char imgpath[PATH_MAX];
-  char fbpath[PATH_MAX];
+  char imgpath_[sizeof ("fb.xyz.") + 2 * 3 * sizeof (int)];
+  char *imgpath = imgpath_;
+  char *fbpath; /* Statically allocate string is returned. */
   char *execargs = NULL;
-  void *new;
   long width, height;
-  size_t size = PATH_MAX;
-  int i, saved_errno;
+  int i, rc = 0, saved_errno = 0;
   
   /* Get pathname for framebuffer, and stop if we have read all existing ones. */
-  get_fbpath (fbpath, try_alt_fbpath, fbno);
+  fbpath = get_fbpath (try_alt_fbpath, fbno);
   if (access (fbpath, F_OK))
     return 1;
   
@@ -367,8 +375,11 @@ save_fb (int fbno, int raw, const char *filepattern, const char *execpattern)
 	sprintf (imgpath, "fb%i.%s.%i", fbno, (raw ? "pnm" : "png"), i);
     }
   else
-    if (evaluate (imgpath, (size_t)PATH_MAX, filepattern, fbno, width, height, NULL) < 0)
-      return -1;
+    {
+      imgpath = evaluate (filepattern, fbno, width, height, NULL);
+      if (imgpath == NULL)
+	goto fail;
+    }
   
   /* Take a screenshot of the current framebuffer. */
   if (save (fbpath, imgpath, width, height, raw) < 0)
@@ -380,29 +391,66 @@ save_fb (int fbno, int raw, const char *filepattern, const char *execpattern)
     return 0;
   
   /* Get execute arguments. */
- retry:
-  new = realloc (execargs, size * sizeof (char));
+  execargs = evaluate (execpattern, fbno, width, height, imgpath);
   if (execargs == NULL)
     goto fail;
-  execargs = new;
-  if (evaluate (execargs, size, execpattern, fbno, width, height, imgpath) < 0)
-    {
-      if ((errno != ENAMETOOLONG) || ((size >> 8) <= PATH_MAX))
-	goto fail;
-      size <<= 1;
-      goto retry;
-    }
   
   /* Run command over image. */
   if (exec_image (execargs) < 0)
     goto fail;
-  free (execargs);
-  return 0;
+  
+  goto cleanup;
   
  fail:
   saved_errno = errno;
+  rc = -1;
+ cleanup:
   free (execargs);
-  errno = saved_errno;
+  if (imgpath != imgpath_)
+    free (imgpath);
+  return errno = saved_errno, rc;
+}
+
+
+/**
+ * Take a screenshot of all framebuffers.
+ * 
+ * @param   raw          Save in PNM rather than in PNG?.
+ * @param   filepattern  The pattern for the filename, `NULL` for default.
+ * @param   execpattern  The pattern for the command to run to
+ *                       process thes image, `NULL` for none.
+ * @return               Zero on success, -1 on error, 1 if no framebuffer exists.
+ */
+static
+int save_fbs (int raw, const char *filepattern, const char *exec)
+{
+  int r, fbno, found = 0;
+  
+ retry:
+  /* Take a screenshot of each framebuffer. */
+  for (fbno = 0;; fbno++)
+    {
+      r = save_fb (fbno, raw, filepattern, exec);
+      if (r < 0)
+	goto fail;
+      else if (r == 0)
+	found = 1;
+      else if (fbno > 0)
+	break;
+      else
+	continue; /* Perhaps framebuffer 1 is the first. */
+    }
+  
+  /* Did not find any framebuffer? */
+  if (found == 0)
+    {
+      if (try_alt_fbpath++ < alt_fbpath_limit)
+	goto retry;
+      return 1;
+    }
+  
+  return 0;
+ fail:
   return -1;
 }
 
@@ -436,7 +484,7 @@ main (int argc, char *argv[])
 #define USAGE_ASSERT(ASSERTION, MSG)  \
   do { if (!(ASSERTION))  EXIT_USAGE (MSG); } while (0)
   
-  int fbno, r, found = 0, raw = 0;
+  int r, raw = 0;
   char *exec = NULL;
   char *filepattern = NULL;
   struct option long_options[] =
@@ -482,30 +530,12 @@ main (int argc, char *argv[])
       filepattern = argv[optind++];
     }
   
- retry:  
   /* Take a screenshot of each framebuffer. */
-  for (fbno = 0;; fbno++)
-    {
-      r = save_fb (fbno, raw, filepattern, exec);
-      if (r < 0)
-	goto fail;
-      else if (r == 0)
-	found = 1;
-      else if (fbno > 0)
-	break;
-      else
-	continue; /* Perhaps framebuffer 1 is the first. */
-    }
-  
-  /* Did not find any framebuffer? */
-  if (found == 0)
-    {
-      if (try_alt_fbpath++ < alt_fbpath_limit)
-	goto retry;
-      
-      print_not_found_help ();
-      return 1;
-    }
+  r = save_fbs (raw, filepattern, exec);
+  if (r < 0)
+    goto fail;
+  if (r > 0)
+    goto no_fb;
   
   /* Warn about being inside a display server. */
   if (have_display ())
@@ -523,5 +553,8 @@ main (int argc, char *argv[])
 	     execname, strerror (errno), failure_file);
   return 1;
   
+ no_fb:
+  print_not_found_help ();
+  return 1;
 }