On 04/26/2015 03:50 PM, H.J. Lu wrote:
> On Fri, Apr 24, 2015 at 8:36 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 02/18/2015 11:46 AM, Florian Weimer wrote:
>>> On 02/16/2015 11:51 PM, Paul Eggert wrote:
>>>> Florian Weimer wrote:
>>>>> So I'm not sure what to do here. Get rid of the alloca? That's going
>>>>> to be more difficult to review.
>>>>
>>>> I haven't read the code carefully, but if the only reason for the alloca
>>>> is to have a temporary string that one can munge by storing '\0' bytes
>>>> at strategic locations, then I presume that one could rewrite the code
>>>> to avoid the need to make a temporary copy,
>>>
>>> Indeed. I introduced __tzstring_len to avoid the need for the copy, and
>>> broke down __tzset_parse_tz into several smaller functions. Hopefully,
>>> the control flow is more transparent.
>>
>> I've committed this now, with a few whitespace fixes.
>>
>
> This caused:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=18333
Fixed thusly. I didn't see the crash in my original testing.
From cc8dcf96e71dd643f929e32150904cd6ad69efa8 Mon Sep 17 00:00:00 2001
Message-Id: <cc8dcf96e71dd643f929e32150904cd6ad69efa8.1430144442.git.fweimer@redhat.com>
From: Florian Weimer <fweimer@redhat.com>
Date: Mon, 27 Apr 2015 15:41:03 +0200
Subject: [PATCH] test-skeleton: Support temporary files without memory leaks
[BZ#18333]
To: libc-alpha@sourceware.org
add_temp_file now makes a copy which is freed by delete_temp_files.
Callers to create_temp_file can now free the returned file name to
avoid the memory leak. These changes do not affect the leak behavior
of existing code.
Also address a NULL pointer derefence in tzset after a memoru allocation
failure, found during testing.
---
ChangeLog | 11 +++++++++++
NEWS | 2 +-
test-skeleton.c | 23 ++++++++++++++++++-----
time/tzset.c | 7 ++++++-
4 files changed, 36 insertions(+), 7 deletions(-)
@@ -1,3 +1,14 @@
+2015-04-27 Florian Weimer <fweimer@redhat.com>
+
+ [BZ#18333]
+ * time/tzset.c (parse_tzname): Return error on memory allocation
+ failure.
+ * test-skeleton.c (struct temp_name_list): Change type of name
+ member to non-const.
+ (add_temp_file): Create a copy of the file name.
+ (delete_temp_files): Deallocate memory.
+ (create_temp_file): Add comment.
+
2015-04-24 Florian Weimer <fweimer@redhat.com>
* io/posix_fallocate.c (posix_fallocate): Do not set errno.
@@ -16,7 +16,7 @@ Version 2.22
17967, 17969, 17978, 17987, 17991, 17996, 17998, 17999, 18019, 18020,
18029, 18030, 18032, 18036, 18038, 18039, 18042, 18043, 18046, 18047,
18068, 18080, 18093, 18100, 18104, 18110, 18111, 18128, 18138, 18185,
- 18197, 18206, 18210, 18211, 18247, 18287.
+ 18197, 18206, 18210, 18211, 18247, 18287, 18333.
* Cache information can be queried via sysconf() function on s390 e.g. with
_SC_LEVEL1_ICACHE_SIZE as argument.
@@ -73,7 +73,7 @@ static const char *test_dir;
struct temp_name_list
{
struct qelem q;
- const char *name;
+ char *name;
} *temp_name_list;
/* Add temporary files in list. */
@@ -83,14 +83,17 @@ add_temp_file (const char *name)
{
struct temp_name_list *newp
= (struct temp_name_list *) calloc (sizeof (*newp), 1);
- if (newp != NULL)
+ char *newname = strdup (name);
+ if (newp != NULL && newname != NULL)
{
- newp->name = name;
+ newp->name = newname;
if (temp_name_list == NULL)
temp_name_list = (struct temp_name_list *) &newp->q;
else
insque (newp, temp_name_list);
}
+ else
+ free (newp);
}
/* Delete all temporary files. */
@@ -100,11 +103,19 @@ delete_temp_files (void)
while (temp_name_list != NULL)
{
remove (temp_name_list->name);
- temp_name_list = (struct temp_name_list *) temp_name_list->q.q_forw;
+ free (temp_name_list->name);
+
+ struct temp_name_list *next
+ = (struct temp_name_list *) temp_name_list->q.q_forw;
+ free (temp_name_list);
+ temp_name_list = next;
}
}
-/* Create a temporary file. */
+/* Create a temporary file. Return the opened file descriptor on
+ success, or -1 on failure. Write the file name to *FILENAME if
+ FILENAME is not NULL. In this case, the caller is expected to free
+ *FILENAME. */
static int
__attribute__ ((unused))
create_temp_file (const char *base, char **filename)
@@ -132,6 +143,8 @@ create_temp_file (const char *base, char **filename)
add_temp_file (fname);
if (filename != NULL)
*filename = fname;
+ else
+ free (fname);
return fd;
}
@@ -201,7 +201,12 @@ parse_tzname (const char **tzp, int whichrule)
if (*p++ != '>' || len < 3)
return false;
}
- tz_rules[whichrule].name = __tzstring_len (start, len);
+
+ const char *name = __tzstring_len (start, len);
+ if (name == NULL)
+ return false;
+ tz_rules[whichrule].name = name;
+
*tzp = p;
return true;
}
--
2.1.0