Page 3 of 10 FirstFirst 12345 ... LastLast
Results 21 to 30 of 99

Thread: How NOT to write a shared library

  1. #21
    Join Date
    Jun 2006
    Location
    The Netherlands
    Beans
    2,185
    Distro
    Ubuntu 12.04 Precise Pangolin

    Re: How NOT to write a shared library

    Quote Originally Posted by j_g View Post
    Alerting people to a potential problem is the best way to avoid it. There's an old saying that an ounce of prevention is worth a pound of cure. That is the "solution".
    Yes, but you should be alerting the author of PulseAudio, and why do you say that this is "not a bug"? Ofcourse it's a bug. And even if it isn't a real mistake in the code, then it's a design bug. It doesn't really matter if you want to call it a bug or not. It's a problem and the author of PulseAudio must be made aware of it.

    Shared libraries must NEVER terminate an application. I'm a Java programmer and doing System.exit(...) (which kills the JVM and so terminates your application) in a library is a deadly sin - especially if you're writing a Java web application that's running in an application server. Doing a System.exit(...) would kill the whole app server.
    Last edited by jespdj; November 15th, 2007 at 01:39 PM.
    Ubuntu 12.04

  2. #22
    Join Date
    Jun 2006
    Location
    CT, USA
    Beans
    5,267
    Distro
    Ubuntu 6.10 Edgy

    Re: How NOT to write a shared library

    First, i appreciate you wrote this manual and try to alert about possible error.
    Quote Originally Posted by j_g View Post
    Alerting people to a potential problem is the best way to avoid it. There's an old saying that an ounce of prevention is worth a pound of cure. That is the "solution".
    I agree, alerting people is **much** better solution than shooting them left and right.

    If we decide to go with shooting (which I advice against), my suggestion would be to shoot people who complain and whine but do not report the bug properly, and who know how to do it right but don't want to share with people who strive to implement free code (while learning to do it right). So after couple generations, we have population of developers who report bugs properly and without whining, and share knowledge with beginners.

    Most people were not born with genetically inherited deep knowledge of how to manage memory allocation errors in C, and shooting people who don't manage memory properly will thin population of developers significantly - possibly leaving only descendants of OP (assuming OP is the only know exception, at least this what I deduced from the rant ) if this trait is indeed inheritable and dominant - and inheritability is not proven yet, or is it? Better not take chances.

  3. #23
    Join Date
    Mar 2005
    Beans
    6,040

    Re: How NOT to write a shared library

    Quote Originally Posted by j_g
    Well, it's not a bug (and I'm sure the original author doesn't consider it to be so, because he deliberately coded it to do what it does). It's simply a design that I consider to be lacking in stability and usability.
    If you think a particular design decision is problematic to the degree that it will indirectly cause crashes and other unwanted behaviour, that's a valid reason to file a bug.

    Quote Originally Posted by j_g
    I posted my objection in a forum here that is supposed to be specifically for providing the Ubuntu developers "feedback" about what features to add to Ubuntu.
    If you mean the Idea Pool, it's not a forum that most Ubuntu developers read. The proper way to contact developers and discuss issues such as this is to post to ubuntu-devel or ubuntu-devel-discuss.

    Quote Originally Posted by j_g
    I would hope that someone on the Ubuntu developer team occasionally reads programming forums here, if to do nothing more than find out what app developers do, and don't, find troublesome, helpful, easy-to-use, difficult, worrisome, etc.
    Yes, probably some developers scan through the forums occasionally, but all of them read the mailing lists all the time, and they check their bug mail all the time. Posting to forums is a very inefficient way to notify developers of issues such as this.

    Quote Originally Posted by j_g
    Alerting people to a potential problem is the best way to avoid it. There's an old saying that an ounce of prevention is worth a pound of cure. That is the "solution".
    True. But you should choose the right medium to alert the right people.

  4. #24
    Join Date
    Feb 2007
    Beans
    236

    Re: How NOT to write a shared library

    Quote Originally Posted by jespdj View Post
    It's a problem and the author of PulseAudio must be made aware of it.
    But surely someone on the Fedora team has already done so (assuming they felt it was a problem)! Remember that Pulse Audio functions as the "sound server" for all Fedora user mode code, even code that uses other audio APIs (ie, ALSA, ESD, OSS) since Pulse Audio has "emulation layers" for those APIs so that all app's audio I/O ultimately goes through Pulse Audio. So, it affects every Fedora app that uses audio (including any "desktop code"). I'd be incredibly surprised if the Fedora team did not do a complete security audit upon that code before adding it to Fedora, especially since they tout it as a major new feature of the OS. They must have already seen this. (I spotted it within 5 minutes of visiting the web site. I have absolute confidence that Fedora developers would be capable of finding what I found). And then there are all these SUSE and Ubuntu developers who have officially acknowledged that they're considering Pulse Audio for inclusion in future releases. Surely, their audit of the code has gone at least as far as my 5 minute inspection. I'm really, really not being facetious here. I genuinely would be shocked if I'm the first, let alone only, person out of all of these developers to notice this. That just doesn't seem possible.

    So realistically, I must assume that either these other developers do not consider it a problem, and/or the Pulse Audio authors do not consider it a problem or do not want to modify the code. In this case, the only alternative is to make people aware of the problem so that:

    1) People can object to the inclusion of Pulse Audio, as it is currently designed, in future distro releases.

    2) People writing shared libs can be made aware that there are folks who notice things like this, and really do look very unfavorably upon such coding. The hope is that others will not do the same thing in their shared libs.

    Code:
    Shared libraries must NEVER terminate an application.
    Absolutely. But there it is. In Fedora 8. Soon to be in SUSE, I hear. And being considered for the next version of Ubuntu. (Very probably other distros as well, if these 3 are already leaning toward it). You're not suggesting that the Fedora 8 devs did not do a security audit upon some major addition that has far-reaching affects (ie, can impact all user mode code that employs audio), or that such an audit was carelessly done such that they were unaware they added a shared lib that is deliberately designed to terminate an app?

    I'm surely not suggesting it. I'm just saying that it was a bad decision to code Pulse Audio that way, and it was a bad (albeit fully cognizant) decision to include it in a distro despite that design flaw, and this same mistake should not be repeated by others.
    Last edited by j_g; November 16th, 2007 at 12:09 AM.

  5. #25
    Join Date
    Aug 2005
    Beans
    73

    Re: How NOT to write a shared library

    Code:
    void pa_xfree(void *p) {
        if (!p)
            return;
    
        free(p);
    }
    That's a nice way of hiding bugs. I would prefer getting slapped by glibc when I free NULL pointers.

    EDIT: free on GNU ignores NULL, but it is still a bad thing as it can hide actual bugs.
    Last edited by tageiru; November 15th, 2007 at 07:24 PM.
    Touting "community" as a strength of a distribution is the equivalent of stating that it has poor documentation.

  6. #26
    Join Date
    Jul 2005
    Beans
    1,535
    Distro
    Ubuntu 8.04 Hardy Heron

    Re: How NOT to write a shared library

    Quote Originally Posted by tageiru View Post
    That's a nice way of hiding bugs. I would prefer getting slapped by glibc when I free NULL pointers.
    There's nothing wrong with freeing null pointers. From the man page:
    free() frees the memory space pointed to by ptr, which must have been returned by a previous call to malloc(), calloc() or realloc(). Otherwise, or if free(ptr) has already been called before, undefined behaviour occurs. If ptr is NULL, no operation is performed
    (emphasis added)

    All those "if(ptr!=NULL)" guards before free/delete calls are useless. Feeing/deleting null pointers is a well defined behavior. Be careful though, freeing or deleting a pointer does not set it to null, so don't double free or delete.
    Last edited by hod139; November 15th, 2007 at 07:25 PM.
    When I invented the Web, I didn't have to ask anyone's permission.
    ~Tim Berners-Lee on Net Neutrality
    -------------------------------------
    Visit the Ubuntu Programming IRC-channel at #ubuntu-programming (chat.freenode.net).

  7. #27
    Join Date
    Feb 2007
    Beans
    236

    Re: How NOT to write a shared library

    Quote Originally Posted by tageiru View Post
    it can hide actual bugs.
    Point taken.

    But at least that won't terminate the calling app. What alarms me is the use of exit() in a shared lib that forms a major component of an operating system (in this case, the user mode audio core. Hmmm. It just occurred to me. Somebody better audit the kernel mode Pulse Audio code too).

    It was a bad decision to put Pulse Audio into Fedora given this current design flaw. And I'm quite sure it was a fully informed decision. This is Fedora we're talking about, not some fly-by-night one-man distro. This is essentially the unstable branch of RHEL. No way would Red Hat let a major component into their distro without a full security audit first. They're not going to put it into Fedora first, then do the audit, and then rip it out if it fails the audit. Someone on the Fedora team knows that exit() is there. They do not need to be told.

    And the Pulse Audio author surely doesn't need ANYONE to tell him he's got calls to exit() in there, and what those do. He knows it. (Well, unless he's totally incompetent, and I'm unwilling to assume that). He designed it that way. Folks need to stop suggesting that someone should tell him about it.

    SOAPBOX ASIDE: Most likely what happened is that he did the error handling this way to "save time" when he was getting the code up and running. It was something "quick and dirty" to get going. No doubt he expected to go back later and add real error handling. But what typically happens in these situations is that the author uses all his time to add new features and the code grows so complex, and so many code paths wind up going through that exit(), that to correct it entails re-editing lots and lots of code. He probably said "The hell with this exit() problem. I'll fix it even later on, after I release the code and people are using it, when I have lots and lots of free time to get rid of it". I tell you folks. Unless you're writing a simple utility, you ultimately DO NOT "save time" by using exit() as "quick and dirty" error handling. You ultimately waste time by going back and having to rewrite too much code that winds its way through that exit(). If you're working on something that may grow into a larger project, you save yourself more time/trouble by doing the error handling correctly the very first time you write the code.

    Now, I'm not sure why the Fedora developers chose to go ahead and employ Pulse Audio as is. Perhaps they just wanted to get a jump on other distros, and felt that the trade-off between claiming an "early exclusive" trumped any potential decrease in stability and usability. After all, you have to first persuade new users to choose your distro before those people can find out about its stability and usability. Maybe the Pulse Audio authors made a promise to Fedora devs that this design flaw would get fixed eventually, and the devs said "Ok, that will be sufficient for us". Maybe if anyone ran into a bit of trouble, the devs would suggest "put more memory in your system", and that would suffice until such time as a fix arrived. Maybe other things happened, or other compromises were made. I'm just guessing here.

    But it was a concious, and I believe bad, decision. And I hope that other distros don't repeat it.

    Furthermore, the whole point of this thread is not to alert the Pulse Audio author to something he already knows about. If that's what any reader thought, then he missed the whole point. The point is to alert programmers who may eventually work on a shared lib, or complex projects, that exit() is a crutch that you should avoid in these situations because it will only give you, and others, problems. Learn from others' mistakes so you don't have to repeat them. Otherwise, 2 years from now, we'll have someone else saying "All these previous audio APIs have flaws". And instead of fixing those (most likely because the other API's authors didn't document their code adequately, and so the new guy figures "I may as well start from scratch. It'll be just as fast and easy"), he'll completely toss away the code bases of OSS, ESD, ALSA, and Pulse Audio, to create yet another sound server code base that repeats the flaws of the previous bases. (Like maybe getting stuck with exit() entwined deep into the bowels of this thing).

    This is not how open source is supposed to work. I thought that the whole point of having source was to fix the bugs, and design flaws/limitations, in the code you're using right now, rather than tossing it away and making new codebases that "re-invent the wheel" with equally bad flaws.

    Come on. I'm a Win32 programmer. I can't be the only one who "gets" how wrong this is???
    Last edited by j_g; November 16th, 2007 at 12:38 AM.

  8. #28
    Join Date
    Dec 2006
    Location
    Uk
    Beans
    109

    Re: How NOT to write a shared library

    Quote Originally Posted by j_g View Post
    And the Pulse Audio author surely doesn't need ANYONE to tell him he's got calls to exit() in there, and what those do. He knows it. (Well, unless he's totally incompetent, and I'm unwilling to assume that). He designed it that way. Folks need to stop suggesting that someone should tell him about it.
    Why do you think the project has a public issue tracker? It's so people who are not the author can see what's going on without spending all day searching forums and mailing lists.
    If you want people to know about an issue with a project then its public tracker is the first port of call.
    OpenStreetMap - Free editable map of the whole world

  9. #29
    Join Date
    Jun 2006
    Location
    CT, USA
    Beans
    5,267
    Distro
    Ubuntu 6.10 Edgy

    Re: How NOT to write a shared library

    I am just curious why you waste so much time complaining and whining and second-guessing motives of Fedora developers here, if you can instead post bug in bug tracker and link this thread to Fedora's developers discussion.

    Either you are smarter than them, and they will fix the bug (and will thank you for finding it), or they are smarted than you, and we will find why it is not a problem.

    What is your REAL goal: have a good reason to complain, or get the bug fixed?
    Last edited by pmasiar; November 16th, 2007 at 05:01 AM.

  10. #30
    Join Date
    Feb 2007
    Beans
    236

    Re: How NOT to write a shared library

    Quote Originally Posted by pmasiar View Post
    I am just curious why you waste so much time complaining and whining and second-guessing motives of Fedora developers here
    I'm curious why you waste so much time repeatedly claiming that all I'm doing is "complaining and whining" when my messages obviously have a great deal of very detailed, specific, technical supporting evidence to back up each and every "complaint" (read: legitimate point) I've made. I'm also curious why you yourself are "second-guessing" my motives to be only "complaining and whining" (with your implication that I allegedly have some sort of hidden agenda), when in the very message above, I've had to come right out and explain to certain people who "don't get it" (ie, keep talking about reporting known design details, to some mailing list) exactly what this thread is all about.

    Perhaps moderators should also be curious.

    Knock it off. I'm not going to complain to the moderators about your personal attacks above, but others around here seem keen to do that. If you want to play that game, that's your prerogative.
    Last edited by j_g; November 16th, 2007 at 08:29 AM.

Page 3 of 10 FirstFirst 12345 ... LastLast

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •