Page 1 of 10 123 ... LastLast
Results 1 to 10 of 99

Thread: How NOT to write a shared library

  1. #1
    Join Date
    Feb 2007
    Beans
    236

    How NOT to write a shared library

    I'm interested in audio, so I'm looking over the (frankly horrible) documentation for this new "Pulse Audio" thing, and I see this function in its shared lib:

    Code:
    pa_xmalloc()
    
    Allocates the specified number of bytes, just like malloc() does.
    
    However, in case of OOM, terminate.
    First of all, why are so many programmers who write a Linux shared library offering a simple wrapper function around malloc? Is it too hard for an app to just call malloc? Is anyone so completely absent-minded that he can't remember malloc is used to allocate memory, so he needs an equivalent wrapper in every shared lib he uses?

    Second of all, in the case of "out of memory", it just terminates your app right then and there??? Oh great, The enduser spends an hour working on something, does some operation, and BANG! -- all his hard work goes down the drain because some shared library decides it wants to terminate the app. Absolutely ridiculous. And I see that other pulse audio functions call this pa_xmalloc too, so good luck getting around it.

    I can just see it now. Every programmer who uses pulse audio will need to add a feature to his app called "Auto-save before every Pulse Audio call because this may be your last chance to preserve your work". Yeah, that should really yield "low latency".

    Sorry, but anyone who puts a call to exit() in a shared library should simply be shot dead. That's really, really crummy, amateurish programming. In a shared lib, if something fails, the professional thing to do is return some error code/signal to the app, indicating that the function failed. Let the app decide what it wants to do. You don't terminate the app on your own.

    And anyone who makes such a shared library a part of his operating system should be shot dead, and then stabbed through the heart with a stake just to make sure he's really dead.

    God help us if this pulse audio thing is ever chosen to replace ALSA in the kernel. This will make Linux stability and reliability go to hell if you have operating system calls terminating apps at will.
    Last edited by j_g; November 15th, 2007 at 12:46 AM.

  2. #2
    Join Date
    May 2007
    Location
    Basildon, England
    Beans
    339
    Distro
    Ubuntu 12.04 Precise Pangolin

    Re: How NOT to write a shared library

    I would like to echo the sentiment of this post.
    In the days when I programmed we often had to fit stuff into the small model (64K code and data) and if we were lucky the next one up (64K code, 64K data).
    That forced you to write efficient code - pointers not indexed arrays etc, functions not inline code etc.
    Now we have almost limitless memory it has led to some poor programming.
    Just to add that of course every function should really return with an termination code. One case I remember the programmer was using long jumps on error leaving all sorts of crap behind when he did so, such as not forcing data writes, not unallocating memory.
    Eh! those were the days!!
    Mick 'n Keef rock, Chas beats time and Ronnie is the new boy
    Registered as user 466848 with the Linux Counter. Registered Ubuntu User 22858. Our company website or our new venture

  3. #3
    Join Date
    Oct 2005
    Location
    UK
    Beans
    242

    Re: How NOT to write a shared library

    Every call to malloc should be checked. So rather than put the checking code at every point you call malloc, you put that code in one function normally named 'xmalloc', and call that instead.

  4. #4
    Join Date
    Oct 2006
    Location
    Austin, Texas
    Beans
    2,712
    Distro
    Ubuntu 7.10 Gutsy Gibbon

    Re: How NOT to write a shared library

    Quote Originally Posted by public_void View Post
    Every call to malloc should be checked. So rather than put the checking code at every point you call malloc, you put that code in one function normally named 'xmalloc', and call that instead.
    But you still need to check for an exception in the calling code. That's the entire reason malloc returns NULL when it fails... That's your exception.

  5. #5
    Join Date
    Apr 2006
    Location
    Phoenix, AZ
    Beans
    251
    Distro
    Ubuntu 8.04 Hardy Heron

    Re: How NOT to write a shared library

    The real kicker here is that you spent 10-20 minutes writing this post. Did you even bother to spend time emailing the author of the library to express your concerns? Probably not. Yeah, everyone should be "shot dead" because how they think something should be done. Give me a break.
    -Skeeterbug

  6. #6
    Join Date
    Feb 2007
    Beans
    236

    Re: How NOT to write a shared library

    Quote Originally Posted by SteveHillier View Post
    the programmer was using long jumps on error leaving all sorts of crap behind when he did so
    Oh yeah, I forgot about the exceedingly evil longjmp(). I actually saw someone use that in recursive code. I was utterly appalled.

    longjmp() is a careless approach toward error handling that only a lazy and negligent programmer would use. Code that uses longjmp is historically buggy and unstable. If someone is using longjmp, he seriously needs to rethink his design.

    <snip>
    Last edited by bapoumba; November 14th, 2007 at 08:12 PM. Reason: snipped out inappropriate comment.

  7. #7
    Join Date
    Feb 2007
    Beans
    236

    Re: How NOT to write a shared library

    Quote Originally Posted by public_void View Post
    Every call to malloc should be checked. So rather than put the checking code at every point you call malloc, you put that code in one function normally named 'xmalloc', and call that instead.
    So you call exit() right then and there, and the hell with preservation of the enduser's hard work, or the reliability of any software that may use your shared lib with exit()'s in it?

    Sorry, but I do not find this technique to be acceptable in terms of reliability, stability, nor even usability. I would never use software that could, at any moment, dump the hard work I had done, or make running apps just disappear at random in a puff of smoke.

    Please rethink your error handling techniques, for the sake of endusers everywhere.

  8. #8
    Join Date
    Feb 2007
    Beans
    236

    Re: How NOT to write a shared library

    Quote Originally Posted by skeeterbug View Post
    You spent 10-20 minutes writing this post. Did you even bother to spend time emailing the author of the library to express your concerns?
    If the pulse audio author actually had his software peer-reviewed before it was put into Fedora, and nobody had balked at this, then I have no faith whatsoever in his peer-review process. A shared lib doesn't need to terminate an app that uses it. ALSA doesn't do it, and there's lots of malloc calls there. Calling exit() in a shared lib is simply sloppy, negligent error handling. No experienced programmer capable of writing quality code should need to be told this. He should already know it.

  9. #9
    Join Date
    Oct 2005
    Location
    UK
    Beans
    242

    Re: How NOT to write a shared library

    So you call exit() right then and there, and the hell with preservation of the enduser's hard work, or the reliability of any software that may use your shared lib with exit()'s in it?
    I never said you should call exit(). I agree you should not call exit() in a library. However you could put error handling code in xmalloc meaning you don't have to put that code throughout your program. The error handling code may be simple such as printing to stderr. Despite having to check the return value of xmalloc, the point is to not to have code repetition to handle the same error after every call.

    Edit: After reading the code (xmalloc.c) the author does write to stderr.
    Last edited by public_void; November 14th, 2007 at 10:37 PM.

  10. #10
    Join Date
    Oct 2006
    Location
    Austin, Texas
    Beans
    2,712
    Distro
    Ubuntu 7.10 Gutsy Gibbon

    Re: How NOT to write a shared library

    Quote Originally Posted by public_void View Post
    I never said you should call exit(). I agree you should not call exit() in a library. However you could put error handling code in xmalloc meaning you don't have to put that code throughout your program. The error handling code may be simple such as printing to stderr. Despite having to check the return value of xmalloc, the point is to not to have code repetition to handle the same error after every call.
    I think the biggest point is that malloc has error handling, it returns NULL. That's a fine exception that your code can catch. I guess it could print to stderr, but even then I think it would be better to say "unable to allocate audio track 'file.ogg'" as opposed to "allocation error". The first is only possible if the calling code catches the exception.

Page 1 of 10 123 ... 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
  •