Re: [PATCH] fdopendir(3)

Kostik Belousov wrote:
[...]
>> Some observations.  __fdopendir2().  My feeling is that this is not 
>> suitable to separate as a standard alone file as its sole users are 
>> fdopendir() and opendir(), therefore, being static might be more 
>> appropriate.  Do you have the intention to use it in somewhere else?  If 
> 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.

I agree most parts of your point, but I would say that by separating the 
function out to its own file, its symbol would be exposed if we provide 
libc.a (but it's right that .so would not be affected).

For the sake of reducing statically linked program's size, will it be 
more helpful to split fts.c into several .c's in order to reduce the 
potentially imported symbols if we statically link the binary, as 
fdopendir() is only an interface translator which is simply a return 
statement, I would say that the affect would be small as compared to 
other parts of libc.

Therefore I still feel that this is more or less an overkill to separate 
into individual files, especially when we do not intend to use 
__fdopendir2 in other parts of libc...

>> so we should really repocopy opendir.c to __fdopendir2.c (there is a 
>> minor unnecessary.
> I do intent to repocopy it. This cannot be represented in the patch.
> 
>> Another thing is that the fd == -1 && (flags & DTF_REWIND) statement. 
>> If this would be an internal routine then it sounds like to be better 
>> represented as an assertion.  The caller should guarantee that the 
>> assertion hold true (by design), and the runtime check seems to be 
>> unnecessary (that's why I did not added these checks).
> Agree.
> 
>> So I think the major difference between your version and mine is whether 
>> 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 
> 000c1160 t __fdopendir2
> As you see, the symbol is local.


-- 
Xin LI <delphij_at_delphij.net>	http://www.delphij.net/
FreeBSD - The Power to Serve!