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 <egg...@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
