On Thu, Oct 8, 2009 at 4:09 AM, Julien Danjou <jul...@danjou.info> wrote: > At 1253709699 time_t, Samuel Thibault wrote: >> Here is an updated patch. Autoreconf needed of course. > > Can someone review this and say yes/no/wtf? :)
Sorry! I didn't notice this bug. Thanks for the reminder, Julien. I'm not yet saying "no" or "wtf" :-) but I'd like to see justification for these patches. "Because glibc does it" is not that convincing. Most convincing would be a pointer to a library that should work in both single-threaded and multi-threaded programs, and needs these functions when running multi-threaded, but where these stubs are appropriate for single-threaded use. It isn't obvious to me that most of the proposed additions are useful, which is why real use cases are important. Here are some specific concerns, as well as which parts of the proposal I approve of as-is. Since we don't provide pthread_create, functions operating on pthread_attr_t don't seem useful. Why would a library that doesn't care about threads ever use them? By the same token, though, pthread_condattr_init and _destroy are a good idea to include. I'd support a patch for that. For either kind of attribute object, it's hard for me to believe that returning 0 from the getter functions is a good idea. That's supposed to indicate success, but the out-parameters won't have been set, so the caller will get garbage. I'd be inclined to not provide any getters. I have no objection to providing setter functions though. The tricky part is that we probably ought to provide *precisely* the collection of setters that the platform's libpthread provides. We certainly should never provide any that the platform does *not* have. In hindsight, I'm concerned about the fact that we provide pthread_cond_wait as a stub returning 0. If it's ever called in a single-threaded application, that function should probably call abort(). The real implementation would be guaranteed to hang in that circumstance, after all. In XCB, we need this to be available as a stub, but I can prove that it won't be called unless there are really multiple threads. So adding pthread_cond_timedwait now has the same issue, except that I suppose a crazy person could use it to sleep for a specified amount of time. In that case, though, immediately returning 0 would be the wrong implementation. I'd rather not provide any implementation until there's a real library with a sane use case for it. Providing a pthread_exit that just calls exit seems fine judging by the POSIX spec, except that the spec says it should call exit(0), not exit(EXIT_SUCCESS). Just because EXIT_SUCCESS==0 on every sane platform doesn't mean we should assume it, right? The pthread_getschedparam and pthread_setschedparam functions don't seem usable in a single-threaded program to me, but if they are they probably shouldn't be implemented as no-ops returning success. I could imagine two possibilities: either return ESRCH, because the thread argument must be invalid, or delegate to sched_setscheduler/sched_getscheduler with a pid of 0, and return -errno on failure. Since pthread_setcancelstate and pthread_setcanceltype are both getters and setters, I'm not sure what a correct stub implementation should do. I'd like to see a real use case for this too. Samuel, thanks for providing these patches. I just need to see some good justification for them. Jamey -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
