GNU bug report logs - #9762
tac fails when given multiple non-seekable inputs due to misuse of mkstemp()

Previous Next

Full log

Message #32 received at 9762 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 9762 <at> debbugs.gnu.org
Subject: Re: bug#9762: tac fails when given multiple non-seekable inputs due
	to misuse of mkstemp()
Date: Tue, 18 Oct 2011 07:46:33 +0200

Paul Eggert wrote:

> On 10/17/11 08:18, Jim Meyering wrote:
>>> +      char const * const Template = "tacXXXXXX";
>
> That "const * const" prompted me to suggest a minor
> improvement.  Since Template's address is never taken,
> better would be
>
>   char const Template[] = "tacXXXXXX";
>
> Hmm, or better yet, get rid of Template entirely:
>
> --- a/src/tac.c
> +++ b/src/tac.c
> @@ -430,12 +430,11 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file)
>
>    if (template == NULL)
>      {
> -      char const * const Template = "tacXXXXXX";
>        tempdir = getenv ("TMPDIR");
>        if (tempdir == NULL)
>          tempdir = DEFAULT_TMPDIR;
>
> -      template = file_name_concat (tempdir, Template, NULL);
> +      template = file_name_concat (tempdir, "tacXXXXXX", NULL);
>      }
>
>    /* FIXME: there's a small window between a successful mkstemp call

I wrote the log for you, and while doing that realized that I found
the following clearer still: (and shorter)

      char *t = getenv ("TMPDIR");
      tempdir = t ? t : DEFAULT_TMPDIR;
      template = file_name_concat (tempdir, "tacXXXXXX", NULL);

With your name on it, I'll wait for an ACK before pushing:

From 385634c8dd512fc4fae46310266247b8fcf2a85a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert <at> cs.ucla.edu>
Date: Tue, 18 Oct 2011 07:43:58 +0200
Subject: [PATCH] maint: make tac.c slightly cleaner

* src/tac.c (copy_to_temp): Now that the template string tacXXXXXX
is used in only one place, don't bother using a separate variable.
Also, using three unconditional assignments seems slightly clearer.
---
 src/tac.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/tac.c b/src/tac.c
index 97b19ae..7d99595 100644
--- a/src/tac.c
+++ b/src/tac.c
@@ -430,12 +430,9 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file)

   if (template == NULL)
     {
-      char const * const Template = "tacXXXXXX";
-      tempdir = getenv ("TMPDIR");
-      if (tempdir == NULL)
-        tempdir = DEFAULT_TMPDIR;
-
-      template = file_name_concat (tempdir, Template, NULL);
+      char *t = getenv ("TMPDIR");
+      tempdir = t ? t : DEFAULT_TMPDIR;
+      template = file_name_concat (tempdir, "tacXXXXXX", NULL);
     }

   /* FIXME: there's a small window between a successful mkstemp call
--
1.7.6.4

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.