4 ms·

I began to have serious concerns about this article when I saw the author allocate and then immediately leak memory in the first example. I agree with the mess

• scott_s 16y ago
  He also elided basic error checking. I have no problem if people don't follow all good programming practices in pedagogical examples.
  • dfox 16y ago
    I would say that the tok = calloc(); tok = strtok(something-which-is-not-in-any-way-related-to-tok) is to some extent clear sign of cargo-cult C programming and has nothing to do with good programming practices.
    • scott_s 16y ago
      I thought the complaint was that he didn't free what he malloced. On second look, yes, that's not the kind of code I want to see anywhere, pedagogical or not.
  • tptacek 16y ago
    What error checking are you referring to? If it's the alloc, I think the explicit error check on it is usually an antipattern and should be avoided.
    • js2 16y ago
      Curious: you think it's better to crash attempting to use a NULL pointer? If so, why is that better than checking for the NULL?
      • tptacek 16y ago
        I think regimes that involve manual checking introduce the possibility of mistakes; when those mistakes involve array references, they can be exploitable. I've always, in both my dev career and in security research, felt that manual checking is dumb. In 99.999% of cases, the only option given to code when allocations fail is to start a chain of events that ends the program. Inflating each and every allocation (or, more likely, missing the little things that allocate silently, like strdup) into 4-5 lines of code seems wasteful. What I advocate is what my friend Danny told me he did at Juno in the mid '90s: preload a wrapper on malloc (don't use silly wrappers like "xmalloc") that catches faults and does something intelligent; centralize your handling of allocation failures, instead of trying to graft it onto each and every allocation point. The nice thing about my philosophy: it means that when you write normal, non-runtime code, you just pretend malloc never fails. Your code is cleaner, and it's probably safer.
        • __david__ 16y ago
          It depends what kind of program it is. A long running GUI program probably doesn't want to completely abort if the user tries to open file that it doesn't have enough memory to process--especially if there are other unsaved documents open. Similarly an embedded program might not want to reboot itself if there aren't enough memory resources to carry out a command (due to other commands being processed at the same time). But you are right, a lot of the time (perhaps even the majority of the time) it really is perfectly acceptable to just die if you can't malloc(). And it really does take a lot of effort to manage memory like that, but if it is done right it can really make things work robustly.
          • tptacek 16y ago
            Your allocation failure strategy is orthogonal to how individual allocations should be handled. In a UI program, keep a reserve of memory, return valid addresses to the caller, and pop up a warning. It's actually easier to do things like this if you aren't hand-crafting individual little alloc-check-handle routines.
  • jsolson 16y ago
    I wasn't actually talking about error checking, I was talking about the lines: char *tok = (char *)malloc(SOMEMEMORY); tok = strtok(data, ":"); I don't actually care whether the malloc succeeded or whether or not he checked for an error, as the second line blows away the only reference to the potentially allocated memory. Whatever may or may not have been allocated has now officially been leaked.
    • tptacek 16y ago
      The cast on that malloc drives me f'ing nuts. DON'T CAST MALLOC.