4 ms·
anti-pattern known sometimes as "defensive programming": If something goes wrong, pretend you didn't notice and try to keep going anyway No. Defensive programm
by tbrownaw 11y ago
anti-pattern known sometimes as "defensive programming": If something goes wrong, pretend you didn't notice and try to keep going anyway
No. Defensive programming means, be predictable even in the face of nonsense. Return an error code instead of segfaulting, define reasonable non-error behavior for thing that don't have to be errors, etc. Sweeping things under the rug just means they'll come back to bite you later, which isn't defensive at all.
and when faced by a choice between making the presence of a bug immediately obvious or trying to hide the bug and potentially exposing serious security vulnerabilities... well, I know which I would prefer
This over a feature that makes "printf debugging" more usable? Is he smoking something?
If the security of your code relies on a parameter not being null, you'd be a fucking idiot to let "printf() should segfault" be the way you catch that. Check it explicitly; not only is it safer but it makes broken callers easier to debug.
- cperciva 11y agoSweeping things under the rug just means they'll come back to bite you later, which isn't defensive at all. I did say "known sometimes as". And I've seen a lot of "check for NULL and silently replace it by something reasonable so that bugs don't make us crash" code justified as being "defensive". If the security of your code relies on a parameter not being null That's not what I said. Exploit paths may however result in pointers being NULL which are normally non-NULL, and so replacing NULL by "(null)" may turn a denial of service into something worse. Check it explicitly I absolutely agree, developers should use more assertions.
- tbrownaw 11y agoThat's not what I said. Exploit paths may however result in pointers being NULL which are normally non-NULL, and so replacing NULL by "(null)" may turn a denial of service into something worse. Exploit paths will likely cause variables to have values outside their expected range. Therefore library functions should be defined as narrowly as possible, in order to convert as many as possible of these cases into crashes. Never mind that this makes the library functions less useful. Never mind that this will push complexity into the caller (or just flat-out add complexity in some cases). Complexity which will necessarily add more opportunities for bugs.
- epidemian 11y ago> Never mind that this will push complexity into the caller (or just flat-out add complexity in some cases). Complexity which will necessarily add more opportunities for bugs. I don't follow how this could happen. Is there any other case where printf printing "(null)" for NULL %s values would be useful besides when debugging? If i am debugging then yes, making printf print "(null)" can be useful. But if instead of that it would segfault, the added complexity of doing an explicit check as in printf("%s", str ? str : "(null)") will not "add more opportunities for bugs", because that code won't live past the debugging session. On the other hand, if i'm not debugging, passing a NULL %s value to printf is almost certainly a bug, and i'd much rather have printf segfaulting in the presence of that bug than it silently ignoring it and printing some flag value.
- skynetv2 11y agoThe behavior is bad because thats not what I expected it to do, so it must be bad. kind of an unreasonable on the authors part
- cperciva 11y agonot what I expected it to do, so it must be bad. Not at all what I said. What I was saying was "actively prevents bugs from being found and fixed, so it must be bad".
- alextgordon 11y agoYou're assuming that NULL represents an error. It doesn't, it represents the lack of a value. Widget w = {}; widget_init(&w); widget_debug_print(&w); void widget_debug_print(Widget* widget) { printf("title: '%s'\n", widget->title); printf("size: (%d, %d)\n", widget->w, widget->h); } I don't know what widget_init() does. It turns out that widget->title is NULL. Great! Now I know. You're basically saying: "In an ideal world, that printf() would be enclosed in an if (), then the code is compliant". OK, but who's going to fix it? The vendor went bankrupt years ago, the code only exits on JAZ tapes, the whole of Uzbekistan relies on it, etc... Relevant: https://lkml.org/lkml/2012/12/23/75 https://lkml.org/lkml/2012/12/23/75
- cperciva 11y agoThe vendor went bankrupt years ago, the code only exits on JAZ tapes, the whole of Uzbekistan relies on it, etc... All the more reason why developers shouldn't be tricked into thinking that their code is correct in the first place.
- kazinator 11y agoThe code isn't incorrect, it is nonportable. If portability isn't a requirement, then it has no bearing on correctness (except that violations of other requirements that do apply to the program may also happen to be nonportable). There is such a thing as nonportable and correct. A program is correct if it implements its specification, and if the specification says "it only has to run on glibc/Linux systems" then passing null pointers to %s is irrelevant. The behavior in this case is undefined, but that only means "not defined by the ISO C standard". In context, it is the opposite to standard-defined, not the opposite to defined. In fact, the given C library has defined the behavior; for the users of that library, it is well-defined: "glibc-defined", if you will. I happen to believe that glibc's definition of behavior is arbitrary and inconsistent, but that's a whole different point from the idea that there should be no defined behavior, or that it should be defined to abort. The %s conversion specifier in glibc chooses a printed representation for the object "null pointer of type char ", the printed representation "(null)" that is not shared by other library functions. For instance, consistency says that we should see the same result if we do strcpy(big_buffer, NULL) and sprintf(big_buffer, "%s", NULL). We do not. One will crash and the other gives us "(null)". Extending behavior in inconsistent ways that breaks basic equivalences and identities is pointless. Moreover, programmers who want (char ) 0 to be "(null)" can do so easily, for instance with an inline function like this: static inline maybenull(const char *s) { returns s ? s : "(null)"; } printf("%s\n", maybenull(str)); The utility of this is low. All it does is waste cycles checking for null, which is of no benefit to a program which never passes null.
- delan 11y agoIt can be difficult to let go of printf(3) debugging, but using a debugger instead can seriously improve your development workflow.
- jeremiep 11y agoNot for multi-threaded or networked code where breaking in the debugger changes the program's behaviour and causes timeouts. I still prefer printf in these cases.
- danieltillett 11y agoMy favourite bug is where adding a printf makes the bug go away. Lots of fun tracking these down in multithreaded code where the bug only shows up after running for a 100 hours.
- nitrogen 11y agoIt's even more fun when it's firmware being debugged via RS232 and the bug only happens during every hundredth bootup :-).
- danieltillett 11y agoI have to say I have never had the pleasure of working at such low level, but I doubt I would enjoy it. Another nice experience I have had is tracking down bugs that only show up in protected/encrypted code (anti-cracking) where you can’t use a debugger - fprintf becomes your best friend here.
- jeremiep 11y agoI don't really enjoy debugging low-level code either, but I absolutely love developing it, reasoning about it and finding solution to problems at that level. Knowing how to code low-level has radically changed how I code high-level as well because I now understand what the potential bottlenecks are on the hardware. Come to think of it, I don't really enjoy debugging no matter what abstraction level I'm at! I prefer taking more time to think carefully about the problem and preventing bugs rather than spending that time chasing them. Which isn't always possible sadly.