Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 23 Jul 2014 14:41:52 -0500
From:      Pedro Giffuni <pfg@freebsd.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r268945 - in head: lib/libc/stdlib sys/sys
Message-ID:  <D3EEEA1E-4225-427E-9AA5-D7002C1AAD3B@freebsd.org>
In-Reply-To: <4953747B-B2B3-4F24-A006-022AEC4DDA29@freebsd.org>
References:  <201407211544.s6LFixKa093406@svn.freebsd.org> <20140723172245.GJ93733@kib.kiev.ua> <4953747B-B2B3-4F24-A006-022AEC4DDA29@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

Il giorno 23/lug/2014, alle ore 12:54, Pedro Giffuni <pfg@FreeBSD.org> =
ha scritto:

>=20
> Il giorno 23/lug/2014, alle ore 12:22, Konstantin Belousov =
<kostikbel@gmail.com> ha scritto:
>=20
>> On Mon, Jul 21, 2014 at 03:44:59PM +0000, Pedro F. Giffuni wrote:
>>> Author: pfg
>>> Date: Mon Jul 21 15:44:59 2014
>>> New Revision: 268945
>>> URL: http://svnweb.freebsd.org/changeset/base/268945
>>>=20
>>> Log:
>>> Fix hdestroy() compliance issue.
>>>=20
>>> The hcreate(3) implementation and related functions we inherited
>>> from NetBSD used to free() the key value, something that is not
>>> supported by the standard implementation.
>>>=20
>>> This would cause a segmentation fault when attempting to run
>>> the examples from the opengroup and linux manpages.  NetBSD
>>> has added non-standard calls to provide the previous
>>> behaviour but hdestroy is not very commonly used so at this
>>> time it seems excessive to bring those to FreeBSD.
>>>=20
>>> Bump the __FreeBSD_version as this is an ABI change.
>>>=20
>>> Reference:
>>> http://bugs.dragonflybsd.org/issues/1398
>>>=20
>>> MFC after:	2 weeks
>>>=20
>>> Modified:
>>> head/lib/libc/stdlib/hcreate.c
>>> head/sys/sys/param.h
>>>=20
>>> Modified: head/lib/libc/stdlib/hcreate.c
>>> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
>>> --- head/lib/libc/stdlib/hcreate.c	Mon Jul 21 15:26:52 2014	=
(r268944)
>>> +++ head/lib/libc/stdlib/hcreate.c	Mon Jul 21 15:44:59 2014	=
(r268945)
>>> @@ -159,7 +159,6 @@ hdestroy_r(struct hsearch_data *head)
>>> 		while (!SLIST_EMPTY(&table[idx])) {
>>> 			ie =3D SLIST_FIRST(&table[idx]);
>>> 			SLIST_REMOVE_HEAD(&table[idx], link);
>>> -			free(ie->ent.key);
>>> 			free(ie);
>>> 		}
>>> 	}
>>>=20
>>> Modified: head/sys/sys/param.h
>>> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
>>> --- head/sys/sys/param.h	Mon Jul 21 15:26:52 2014	=
(r268944)
>>> +++ head/sys/sys/param.h	Mon Jul 21 15:44:59 2014	=
(r268945)
>>> @@ -58,7 +58,7 @@
>>> *		in the range 5 to 9.
>>> */
>>> #undef __FreeBSD_version
>>> -#define __FreeBSD_version 1100027	/* Master, propagated to newvers =
*/
>>> +#define __FreeBSD_version 1100028	/* Master, propagated to newvers =
*/
>>>=20
>>> /*
>>> * __FreeBSD_kernel__ indicates that this system uses the kernel of =
FreeBSD,
>> You broke the ABI.  This is absolute stopper for the symversioned
>> fundamental library.
>>=20
>=20
> I thought about it for a while.
> Actually .. I didn=92t. The interface remains unchanged, I only fixed =
an undocumented bug.
>=20

As clarification .. I meant I didn=92t actually break the ABI ...

It=92s just a bug fix and we shouldn=92t provide compatibility shims for =
bugs.

> I say undocumented because while he NetBSD man page did document the =
bug, our man page has always claimed compliance .
>=20
>> The right thing to do is to introduce a new version of hcreate() for =
FBSD_1.4,
>> and provide compat shims with the old behaviour for FBSD_1.0.
>=20
> hcreate() is not used in the base: all consumers (ports tree) are =
likely to expect the standard behavior so this change saves them from a =
segmentation fault. In the rare case of someone having developed their =
code for the broken function, the worst they will see is a memory leak.
>=20
> A compat shim would be really ugly :(.
>=20

FWIW, NetBSD provides new functions with the old behavior but hdestroy() =
now conforms to standards. Things there got really ugly because they =
also had that issue for the re-entrant versions.

I am thinking of MFCing the fix, but not the re-entrant versions.

Pedro.=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?D3EEEA1E-4225-427E-9AA5-D7002C1AAD3B>