Date: Tue, 15 Apr 2008 12:01:39 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: d@delphij.net Cc: FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: [PATCH] fdopendir(3) Message-ID: <20080415090138.GK18958@deviant.kiev.zoral.com.ua> In-Reply-To: <4803B0EC.1060901@delphij.net> References: <48027F56.9010302@delphij.net> <20080414095539.GD18958@deviant.kiev.zoral.com.ua> <4803B0EC.1060901@delphij.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--fLj60tP2PZ34xyqD Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Apr 14, 2008 at 12:30:52PM -0700, Xin LI wrote: > Kostik Belousov wrote: > >On Sun, Apr 13, 2008 at 02:47:02PM -0700, Xin LI wrote: > >>Hi, > >> > >>Any objection for the attached patch which implements fdopendir(3) that= =20 > >>is found in various other OSes? Basically it splits __opendir2 into tw= o=20 > >>parts, and expose the second part which deals with fd to provide=20 > >>fdopendir(3) functionalities. > > > >There are some problems with the DTF_REWIND and union mounts. > > > >I too implemented the fdopendir in the course of the *at() work after > >the initial Roman Divacky submission. I put my patch at the > >http://people.freebsd.org/~kib/misc/fdopendir.1.patch > > > >I postponed the commit for further testing and some more changes related > >to the committed *at syscalls (mainly man pages, the patch awaits the > >review). >=20 > Some observations. __fdopendir2(). My feeling is that this is not=20 > suitable to separate as a standard alone file as its sole users are=20 > fdopendir() and opendir(), therefore, being static might be more=20 > appropriate. Do you have the intention to use it in somewhere else? If= =20 Being static, it must be present in the same source file with the callers. Since the static libraries (libXXX.a) try to avoid bringing in non-used symbols by separating each symbol in the individual source file, I put the __fdopendir2, opendir and fdopendir into the individual files. Yes, use of the opendir or fdopendir would bring the __fdopendir2 in, but use of the opendir would not expose fdopendir extra. > so we should really repocopy opendir.c to __fdopendir2.c (there is a=20 > minor unnecessary. I do intent to repocopy it. This cannot be represented in the patch. >=20 > Another thing is that the fd =3D=3D -1 && (flags & DTF_REWIND) statement.= =20 > If this would be an internal routine then it sounds like to be better=20 > represented as an assertion. The caller should guarantee that the=20 > assertion hold true (by design), and the runtime check seems to be=20 > unnecessary (that's why I did not added these checks). Agree. >=20 > So I think the major difference between your version and mine is whether= =20 > we wanted to expose __fdopendir2() outside libc? Mine __fdopendir2() is also not exposed for the libc users, since it is not present in the public symbol map. nm DEV/src/lib/libc/libc.so.7 | grep __fdopendir2=20 000c1160 t __fdopendir2 As you see, the symbol is local. --fLj60tP2PZ34xyqD Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.8 (FreeBSD) iEYEARECAAYFAkgEbvIACgkQC3+MBN1Mb4jP5wCggh7z9D6A6TnpCTzoqvP8ekR+ DsgAoO+k/hDLn6k+UgDga4o1zm89AeVq =bWMp -----END PGP SIGNATURE----- --fLj60tP2PZ34xyqD--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080415090138.GK18958>