NetBSD Problem Report #50259

From www@NetBSD.org  Sat Sep 19 12:48:48 2015
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id A7655A6531
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 19 Sep 2015 12:48:48 +0000 (UTC)
Message-Id: <20150919124846.90ADFA6554@mollari.NetBSD.org>
Date: Sat, 19 Sep 2015 12:48:46 +0000 (UTC)
From: devnexen@gmail.com
Reply-To: devnexen@gmail.com
To: gnats-bugs@NetBSD.org
Subject: Small memory leaks in localtime's code
X-Send-Pr-Version: www-1.0

>Number:         50259
>Category:       lib
>Synopsis:       Small memory leaks in localtime's code
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Sep 19 12:50:00 +0000 2015
>Last-Modified:  Sun Sep 20 15:15:00 +0000 2015
>Originator:     David CARLIER
>Release:        current
>Organization:
>Environment:
NetBSD  7.99.21 NetBSD 7.99.21 (GENERIC) #0: Sat Sep 19 08:11:51 IST 2015  root@:/usr/obj/sys/arch/amd64/compile/GENERIC amd64

>Description:
Hi all,

localtime uses two internal static states variables which are allocated once. But, they re never freed. This patch suppresses the need of dynamic memory allocation.

Hope it helps.
>How-To-Repeat:

>Fix:
Index: time/localtime.c
===================================================================
RCS file: /cvsroot/src/lib/libc/time/localtime.c,v
retrieving revision 1.97
diff -r1.97 localtime.c
170,171c170,173
< static timezone_t lclptr;
< static timezone_t gmtptr;
---
> static struct state lclmem;
> static struct state gmtmem;
> static timezone_t lclptr = &lclmem;
> static timezone_t gmtptr = &gmtmem;
1295,1302c1297,1300
< 	if (! sp)
< 		lclptr = sp = malloc(sizeof *lclptr);
< 	if (sp) {
< 		if (zoneinit(sp, name) != 0)
< 			zoneinit(sp, "");
< 		if (0 < lcl)
< 			strcpy(lcl_TZname, name);
< 	}
---
> 	if (zoneinit(sp, name) != 0)
> 		zoneinit(sp, "");
> 	if (0 < lcl)
> 		strcpy(lcl_TZname, name);
1337,1339c1335
< 		gmtptr = malloc(sizeof *gmtptr);
< 		if (gmtptr)
< 			gmtload(gmtptr);
---
> 		gmtload(gmtptr);
1529,1530c1525
< 		result->TM_ZONE = offset ? __UNCONST(wildabbr) : gmtptr ?
< 		    gmtptr->chars : __UNCONST(gmt);
---
> 		result->TM_ZONE = offset ? __UNCONST(wildabbr) : gmtptr->chars;
2281,2282c2276
< 	if (lclptr)
< 		t = (time_t)(t - leapcorr(lclptr, t));
---
> 	t = (time_t)(t - leapcorr(lclptr, t));
2323,2324c2317
< 	if (lclptr)
< 		t = posix2time_z(lclptr, t);
---
> 	t = posix2time_z(lclptr, t);

>Audit-Trail:
From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@NetBSD.org
Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org
Subject: re: lib/50259: Small memory leaks in localtime's code
Date: Sun, 20 Sep 2015 08:53:47 +1000

 doesn't this patch mean that every one has this allocated instead
 of only users of localtime(3)?

 seems like the wrong choice to me.


 .mrg.

From: David CARLIER <devnexen@gmail.com>
To: gnats-bugs@netbsd.org, mrg@eterna.com.au
Cc: 
Subject: Re: lib/50259: Small memory leaks in localtime's code
Date: Sun, 20 Sep 2015 05:49:37 +0100

 --001a11c2c56212834b0520267fca
 Content-Type: text/plain; charset=UTF-8

 On 19 September 2015 at 23:55, matthew green <mrg@eterna.com.au> wrote:

 > The following reply was made to PR lib/50259; it has been noted by GNATS.
 >
 > From: matthew green <mrg@eterna.com.au>
 > To: gnats-bugs@NetBSD.org
 > Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org,
 >     netbsd-bugs@netbsd.org
 > Subject: re: lib/50259: Small memory leaks in localtime's code
 > Date: Sun, 20 Sep 2015 08:53:47 +1000
 >
 >  doesn't this patch mean that every one has this allocated instead
 >  of only users of localtime(3)?
 >
 >

 localtime(_r) / gmttime(_r) more precisely, but since some softwares rely
 on this (syslog, nrpd ...), I thought it might be a good "trade-off".



 >  seems like the wrong choice to me.
 >
 >
 I understand no problem.


 >
 >  .mrg.
 >
 >

 --001a11c2c56212834b0520267fca
 Content-Type: text/html; charset=UTF-8
 Content-Transfer-Encoding: quoted-printable

 <div dir=3D"ltr"><br><div class=3D"gmail_extra"><br><div class=3D"gmail_quo=
 te">On 19 September 2015 at 23:55, matthew green <span dir=3D"ltr">&lt;<a h=
 ref=3D"mailto:mrg@eterna.com.au" target=3D"_blank">mrg@eterna.com.au</a>&gt=
 ;</span> wrote:<br><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 =
 .8ex;border-left:1px #ccc solid;padding-left:1ex">The following reply was m=
 ade to PR lib/50259; it has been noted by GNATS.<br>
 <br>
 From: matthew green &lt;<a href=3D"mailto:mrg@eterna.com.au">mrg@eterna.com=
 .au</a>&gt;<br>
 To: gnats-bugs@NetBSD.org<br>
 Cc: <a href=3D"mailto:lib-bug-people@netbsd.org">lib-bug-people@netbsd.org<=
 /a>, <a href=3D"mailto:gnats-admin@netbsd.org">gnats-admin@netbsd.org</a>,<=
 br>
 =C2=A0 =C2=A0 <a href=3D"mailto:netbsd-bugs@netbsd.org">netbsd-bugs@netbsd.=
 org</a><br>
 Subject: re: lib/50259: Small memory leaks in localtime&#39;s code<br>
 Date: Sun, 20 Sep 2015 08:53:47 +1000<br>
 <br>
 =C2=A0doesn&#39;t this patch mean that every one has this allocated instead=
 <br>
 =C2=A0of only users of localtime(3)?<br>
 <br></blockquote><div><br></div><div><br></div><div>localtime(_r) / gmttime=
 (_r) more precisely, but since some softwares rely on this (syslog, nrpd ..=
 .), I thought it might be a good &quot;trade-off&quot;.</div><div><br></div=
 ><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .=
 8ex;border-left:1px #ccc solid;padding-left:1ex">
 =C2=A0seems like the wrong choice to me.<br>
 <br></blockquote><div><br></div><div>I understand no problem.</div><div>=C2=
 =A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;borde=
 r-left:1px #ccc solid;padding-left:1ex">
 <br>
 =C2=A0.mrg.<br>
 <br>
 </blockquote></div><br></div></div>

 --001a11c2c56212834b0520267fca--

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, lib-bug-people@netbsd.org, 
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, devnexen@gmail.com
Cc: 
Subject: Re: lib/50259: Small memory leaks in localtime's code
Date: Sun, 20 Sep 2015 09:51:04 -0400

 On Sep 20,  4:50am, devnexen@gmail.com (David CARLIER) wrote:
 -- Subject: Re: lib/50259: Small memory leaks in localtime's code

 I would like to note for completeness, upstream still has this code
 #ifdef'ed by ALL_STATE. I asked to remove it in order to simplify things
 since it does not save much and they said that it might hurt embedded
 systems. It is pretty simple to put them back if we want, it is a dozen
 lines of code or less.


From: David CARLIER <devnexen@gmail.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/50259: Small memory leaks in localtime's code
Date: Sun, 20 Sep 2015 16:14:26 +0100

 --Apple-Mail=_C6DAE2EC-D408-44A9-BBBC-BAE4F82BEDFF
 Content-Transfer-Encoding: 7bit
 Content-Type: text/plain;
 	charset=us-ascii

 Hi here a different patch with ALL_STATE handling.

 Hope it helps.


 --Apple-Mail=_C6DAE2EC-D408-44A9-BBBC-BAE4F82BEDFF
 Content-Disposition: attachment;
 	filename=patch-lib_libc_time_localtime.diff
 Content-Type: application/octet-stream;
 	name="patch-lib_libc_time_localtime.diff"
 Content-Transfer-Encoding: 7bit

 Index: time/localtime.c
 ===================================================================
 RCS file: /cvsroot/src/lib/libc/time/localtime.c,v
 retrieving revision 1.97
 diff -r1.97 localtime.c
 169a170
 > #ifdef	ALL_STATE
 171a173,178
 > #else
 > static struct state lclmem;
 > static struct state gmtmem;
 > static timezone_t lclptr = &lclmem;
 > static timezone_t gmtptr = &gmtmem;
 > #endif
 344a352
 > #ifdef ALL_STATE
 348a357
 > #endif
 1294a1304
 > #ifdef	ALL_STATE
 1302a1313,1318
 > #else
 > 	if (zoneinit(sp, name) != 0)
 > 		zoneinit(sp, "");
 > 	if (0 < lcl)
 > 		strcpy(lcl_TZname, name);
 > #endif
 1336a1353
 > #ifdef	ALL_STATE
 1339a1357,1359
 > #else
 >                 gmtload(gmtptr);
 > #endif
 1403a1424
 > #ifdef	ALL_STATE
 1407a1429
 > #endif
 1527a1550
 > #ifdef	ALL_STATE
 1530a1554,1557
 > #else
 > 	if (result)
 > 		result->TM_ZONE = offset ? __UNCONST(wildabbr) : gmtptr->chars;
 > #endif
 1597a1625
 > #ifdef	ALL_STATE
 1598a1627,1629
 > #else
 >         i = sp->leapcnt;
 > #endif
 2028a2060
 > #ifdef	ALL_STATE
 2030a2063
 > #endif
 2133a2167
 > #ifdef	ALL_STATE
 2137a2172
 > #endif
 2280a2316
 > #ifdef	ALL_STATE
 2282a2319,2321
 > #else
 > 	t = (time_t)(t - leapcorr(lclptr, t));
 > #endif
 2322a2362
 > #ifdef	ALL_STATE
 2324a2365,2367
 > #else
 > 	t = posix2time_z(lclptr, t);
 > #endif

 --Apple-Mail=_C6DAE2EC-D408-44A9-BBBC-BAE4F82BEDFF
 Content-Transfer-Encoding: 7bit
 Content-Type: text/plain;
 	charset=us-ascii



 --Apple-Mail=_C6DAE2EC-D408-44A9-BBBC-BAE4F82BEDFF--

Home	PR Database Search