From owner-freebsd-current@FreeBSD.ORG Tue Mar 8 21:23:47 2011 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3D1A31065672; Tue, 8 Mar 2011 21:23:47 +0000 (UTC) (envelope-from kabaev@gmail.com) Received: from mail-qw0-f54.google.com (mail-qw0-f54.google.com [209.85.216.54]) by mx1.freebsd.org (Postfix) with ESMTP id C06918FC18; Tue, 8 Mar 2011 21:23:46 +0000 (UTC) Received: by qwj8 with SMTP id 8so4680488qwj.13 for ; Tue, 08 Mar 2011 13:23:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:date:from:to:cc:subject:message-id:in-reply-to :references:x-mailer:mime-version:content-type; bh=p6xIM78FWmtTcnKSBAhJTMDboxBXeP2f49jytgfqxbs=; b=w4K1BN2G5Caqugj/Vzm17qzihKlud7X0h9iEWxCuAUUUBtrLlNz/M1F3c6bSVxPDxR uU9nkM50+ZPQjoeG1vr0HKhs3zcje/rXVaucSyACk9oRq5JCq6Fhml9Jx6mBl70UjdUZ IitJpnU/N81uhuyjXw1mHrTDjC2g8AB6txzL0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:in-reply-to:references:x-mailer :mime-version:content-type; b=dTyM9EfFvfbCDiCVmpByVZKfnAnM12H1uuTZNSYOGBFwchdS+vjmMP68FiMs4zVQzf GQQTVTEANWh2PFjWpse9dXz5XcpbqeWYuaEu6yg/JYcj0+waq7I/20PqD2Je/Q1RODQ8 U+J922ig6bgUwu+NZXwKhXG0KobEB3K5c7Ma8= Received: by 10.229.32.69 with SMTP id b5mr4806230qcd.266.1299617943422; Tue, 08 Mar 2011 12:59:03 -0800 (PST) Received: from kan.dnsalias.net (c-24-63-226-98.hsd1.ma.comcast.net [24.63.226.98]) by mx.google.com with ESMTPS id e29sm833213qck.15.2011.03.08.12.59.00 (version=SSLv3 cipher=OTHER); Tue, 08 Mar 2011 12:59:01 -0800 (PST) Date: Tue, 8 Mar 2011 15:58:54 -0500 From: Alexander Kabaev To: Gabor Kovesdan Message-ID: <20110308155854.3640550a@kan.dnsalias.net> In-Reply-To: <4D66DE70.9000100@kovesdan.org> References: <4D66DE70.9000100@kovesdan.org> X-Mailer: Claws Mail 3.7.8 (GTK+ 2.22.1; amd64-portbld-freebsd9.0) Mime-Version: 1.0 Content-Type: multipart/signed; micalg=PGP-SHA1; boundary="Sig_/9759GAhvcTdBrxkU.zkx5KP"; protocol="application/pgp-signature" Cc: current@FreeBSD.org, i18n@FreeBSD.org Subject: Re: HEADSUP: BSD iconv coming to the base system with default off X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Mar 2011 21:23:47 -0000 --Sig_/9759GAhvcTdBrxkU.zkx5KP Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Thu, 24 Feb 2011 22:40:48 +0000 Gabor Kovesdan wrote: > Hi Folks, >=20 > I've got some time again to keep working on iconv. The last time I=20 > posted a patch, there were problems with some ports but apart from > this it proved to be quite mature, so I decided to commit it to the > base system but the default setting will be disabled. It can be > enabled by setting the WITH_ICONV knob but whoever does it will take > into account that it may break some ports from being built. However, > this change will help me with future work and will also help > interested people in getting involved in the testing. The rather big > changeset is coming soon. >=20 > Regards, > Gabor Kovesdan Hi Gabor, for whatever historic reason I had WITH_ICONV in /etc/make.conf on my desktop, so I got to be your guinea pig without conscious consent for that role on my part. I did hit several issues so far: 1. mutt-devel port does not consider our implementation as 'good enough'. It runs the test below, which returns 1, while GNU libiconv returns 0. | #include | int main() | { | iconv_t cd; | char buf[4]; | char *ob; | size_t obl; | ob =3D buf, obl =3D sizeof(buf); | return ((cd =3D iconv_open("UTF-8", "UTF-8")) !=3D (iconv_t)(-1) && | (iconv(cd, 0, 0, &ob, &obl) || | !(ob =3D=3D buf && obl =3D=3D sizeof(buf)) || | iconv_close(cd))); | } 2. The implementation of internal locking in citrus_locking.h is a strange one. Here is why: Do you really want to have the lock declared as static in header file? Even is so, please note that declaring data in header file is confusing and having three locks with the same name is not making the library any easier to debug. =20 You need to use proper XXX_STATIC_INITIALIZER constant to initialize the lock statically, instead of declaring it as static. The code only works on FreeBSD due to luck and will break if our typedefs for pthreads lock primitives will ever grow to be the proper structures instead of simple pointers. The locking in citrus_mapper.c is broken in case of parallel and sequential mappers. The file grabs the lock and calls _mapper_open. If mapper being loaded happens to be of a composite type, it will in turn try to invoke _mapper_open on subordinate mappers recursively and will deadlock waiting for the lock it already owns. And at last, by are you using rwlock if all locking you ever do is an exclusive one? 3. There are dangerous vestiges of iconv.dir support in citrus_iconv.c file, _citrus_iconv_open function. You call _lookup_alias there using an uninitialized path variable, causing every iconv-using program to try and access .db file. While there, since you are not using iconv.dir, would it make sense to just merge iconv_std and iconv_none into libc? Same goes to various flavors of libmapper_*.so, is thee a reason why each comes in its own trivial shared library? 4. Most of _citrus_ functions are internal interface between encoders and mappers and do nto constitute public ABI we as a project are signing on to support forever. As such, they belong in private namespace and should not pollute FBSD1.2. =20 -- Alexander Kabaev --Sig_/9759GAhvcTdBrxkU.zkx5KP Content-Type: application/pgp-signature; name=signature.asc Content-Disposition: attachment; filename=signature.asc -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.17 (FreeBSD) iD8DBQFNdpiTQ6z1jMm+XZYRAmA0AKCGSP8tZ98anWqWEvqiaJqvCW3hmwCdEZNC 26Ll8TH+6ZYr5QZFoBV2Zus= =XthY -----END PGP SIGNATURE----- --Sig_/9759GAhvcTdBrxkU.zkx5KP--