1. 35
  1. 19

    What to say, what to say. Yes, generally speaking I agree. “Defensive programming” can be like shotgun parsing applied to anything that’s not parsing.

    Checks here, checks there, checks everywhere. Checks, not asserts. Checks that try to do something reasonable (such as avoid crashing the program). It muddies up the logic and gets in the way, and it makes hard to reason about what’s meant to be a valid kind of thing to pass to a function (ideally we’d have types to express that, but I still use mainstreamish programming languages). Is something valid because you can pass it without crashing the program? Or is it potentially invalid, because there is a check that makes the function a no-op or bail-out-with-error or whatever? What if you don’t notice this case and the invalid value just keeps going deeper and deeper into the program?

    However, I also don’t feel like free() being a no-op when given a NULL pointer is necessarily defensive programming. One might say that you should never call free if you haven’t got something to deallocate, but in a program littered with constructs like

    if (ptr) free(ptr);
    

    the “defensive check” is simply moving common, repeated logic into one place. Isn’t that what functions exist for?

    Herein lies the difficulty. It can be hard to decide whether some check is defending against invalid state or just implementing valid logic. I still struggle with it, having been programming for 20 years. If someone’s got a good rule of thumb, I’m all floppy hears. I kinda suspect it’s too context specific for a rule of thumb though.

    1. 2

      if (ptr) free(ptr);

      This isn’t defensive, this is not knowing the tools: free(NULL) is a safe no-op. Double-freeing is bad, but that construct won’t catch that. This construct will prevent it: free(ptr); ptr = NULL;

      There. Now, you can’t double-free, and you can write free(ptr); an arbitrary number of times to make the cleanup code less conditional and more robust.

      1. 1

        free(NULL) is a safe no-op

        Yes, that’s what I said.

        What I’m questioning here is whether moving that check from the place of call to the implementation of free is defensive programming or not. If you RTFA, spc476 tells a story about a function that is essentially free(), except that on some platforms it’s got the check built in and on others it doesn’t.

        Is the built in check defending against the blunder of passing the function a NULL pointer, or is it just a convenient place to lay the logic that would otherwise be repeated all over?

      2. 2

        In my (granted pretty limited) experience if (ptr) free(ptr); is something that either indicates

        • some confusion about how ptr is used (similar issue in python 2 was calling decode on a already decoded string. This would no-op, but would leave people thinking that the source thing is bytes, or let people mix and match bytes and strings). Usually pointers are either freed or not freed so… how are you in a state where you don’t know if something needs to be freed?

        • is actually being used as a sort of “optional type”. The check is similar to null-checking in other languages. Imagine if foo.bar no-oped if foo was null. Seems pretty reasonable to ask the user to confirm this.

        Rust’s linear types work on some level because of the fact that you usually know whether something needs to be freed or not.

        Then again, I don’t have that much experience with C APIs in a while. It could be pretty common to optionally pass in pointers all over. But I feel like it’s not?

        1. 1

          is actually being used as a sort of “optional type”. The check is similar to null-checking in other languages. Imagine if foo.bar no-oped if foo was null. Seems pretty reasonable to ask the user to confirm this.

          This is what is is, generally. If you think of it not as foo.bar() where foo is NULL, but rather as bar(Nothing) where a NULL value stands for nothing, does it make a little more sense?

          If you want to go further, does NULL stand for an empty list, or does it stand for nothing? Does it make sense to free an empty list? Why not?

          There are generally three ways to implement optionality in C apis: (1) format strings, which are used to specify what you have, (2) pointer + count (think arrays), (3) NULL pointer as a stand-in for nothing, or any other pointer to a something. In addition to that, NULL pointers see some use as sentinels to indicate that I have nothing more. All pretty common; any fancier way to indicate optionality gets clumsy.

          Rust’s linear types work on some level because of the fact that you usually know whether something needs to be freed or not.

          How does that knowledge manifest? In C, it is sometimes the case that the place of resource acquisition is near the place of free, so you can encode this knowledge in control flow. But that’s not always the case.. then storing NULL pointers to say that I have nothing is a way to encode the knowledge of (not) having to free something. And when it comes to that, it’s either if (ptr) free(ptr); all over, or the common logic is built into free().

          1. 2

            Thank you for this detailed reply! It helped me to identify something that causes a bit of a mismatch with C programs. In reality, C ends up being applied in two different ways:

            • writing tricky system internals (the stuff where the bit twiddling and the array pointer math is a necessary evil/very useful)
            • writing the same kinds of applications as other people write in higher-level languages

            So in former case certain API designs make sense, but in the latter one another kind of API design makes sense. Of course, the rub is that C does not provide enough abstraction tooling to write “good” API designs for higher level stuff (leading to people designing APIs purely through macros).

            In a lot of higher level software I would prefer for a double free to blow up so that I now can go in and find the issue with the data flow, since the double free problem also has the nasty “use after free” dual problem. But if you’re writing, say, a language interpreter it’s possible that the data flow is so dependent on user data that it’s very difficult to do this in a manageable way. So this more permissive API would end up being more helpful than having your program crash all the time

      3. 7

        I think that’s a good insight that defense programming makes sense at I/O boundaries the most. I do think if you are in an untyped language it helps to create checks at all API boundaries to enforce assumptions.

        1. 6

          It is because of bugs like this that I am not a fan of defensive programming. They can hide. They can fester. They can be a nightmare to debug at 3:00 am on a production system sans a debugger.

          You can replace “defensive programming” with “offensive programming” (or whatever the opposite is) and the statement is still true. In fact, you can replace it with just about any kind of programming and it’s still true in my experience. Bugs can always hide, fester, and be a nightmare to debug.

          1. 3

            I’m not sure that’s the case. My understanding is that the distinction is between validating data at a system boundary vs validating data everywhere.

            I’ve been through plenty of coding interviews where I’m asked to implement some trivial JavaScript function, and then I’ve been told that my implementation is poor because I didn’t check for invalid inputs, e.g. “ohhhh but what if the programmer passes in undefined? Derp derp derp derp”.

            If you have stronger guarantees that the data you’re passing through functions is valid, then you have fewer places for bugs to occur in your system.

            1. 4

              My answer to the “what if the programmer passed in undefined?” is “that would be a bug and the programmer needs to fix the code.” Why is the routine getting undefined? Why was it not detected earlier?

              1. 3

                This is one reason I prefer languages where you simply can’t get a nil unless you explicitly ask for it, through Option<T> or what have you, and then you must explicitly handle it. But even in C, I prefer to place nullability attributes on everything, so that it’s clear where NULL is allowed. Misuse becomes a compile-time error, and failure to check a nullable pointer is flagged by the analyser.

                1. 1

                  How do you add nullability attributes in C? Something to do with __attribute__?

                  1. 4

                    It’s rarely used and possibly cumbersome, but you can use single field structs to bolt more complex types on to functions.

                    struct notnull { void *ptr; };
                    struct maybenull { void *ptr; };
                    

                    Now you have two incompatible types that can’t be accidentally mixed.

                    1. 1

                      Yes. I have a file with macros that contains something like:

                      #ifndef __has_attribute
                      #define __has_attribute(x) 0
                      #endif
                      
                      #if __has_attribute(nonnull)
                      #define NONNULL __attribute__((nonnull))
                      #else
                      #define NONNULL
                      #endif
                      
                      [a bunch of others]
                      
            2. 4

              Maybe I’m just in the minority here but most of my defensive programming is about determining wheni should crash vs when i should take a different code path. I feel like this case was an instance of Solaris being more defensive than MacOs and Linux.

              1. 4

                There’s a big difference between choosing to crash and just .. happening to crash per chance due to UB. Which was it?

              2. 2

                Go for defensive + warning log.

                In that way you can easily find about a consistent incorrect behaviour without having the app explode because of that.

                1. 1

                  Relevant Hoare’s paper: How did software get so reliable without proof? https://www.gwern.net/docs/math/1996-hoare.pdf