“(null)” is a bad idea
daemonology.net
daemonology.net
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.
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.
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.
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.
kind of an unreasonable on the authors part
Not at all what I said. What I was saying was "actively prevents bugs from being found and fixed, so it must be bad".
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
All the more reason why developers shouldn't be tricked into thinking that their code is correct in the first place.
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.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.
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.
I used to think these were a distraction from actual programming until I realized how much debugging time they save.
The next best thing IMO is detecting crashes and printing a stack trace, which reminds me I've been meaning to release some code I wrote that does so for C/glibc programs.
While I'm no fan of the Java language, you get all these benefits for other JVM-hosted languages such as Clojure and Scala and that is fantastic. You basically get the entire ecosystem for free.
However, network time-outs are still an issue when breaking in the debugger while running a multi-player session. We have a few quirks around this, but nothing perfect.
And we also develop on OS X and Linux where VS isn't available :)
Unfortunately it doesn't work so well for things like intermittent wrong behavior in long-running programs. Better in that case to use pervasive logging (aka, printf debugging to a file).
Having "%s" print "(null)" is absolutely useless in the meat of your string manipulation code. It's pretty much guaranteed to be a bug and could easily mask a problem (as described in the article).
So I think the real problem here is that printf() is used for both debugging and for non-debugging string manipulation (sprintf() and friends). It's trying to serve two masters and it can never get both cases right. Maybe we should make "%s" not check for null and add a null-able "%S" for debugging.
printf("name: %s %s", first, last);
If it crashes, there's no easy way to know which one was NULL!Sorry, but I agree with all the folks who say printf (and similar) is categorically more useful when it prints "(null)". I've had many obscure crashes on systems that were precisely because a logging printf in a seldom-tested piece of code had a parameter that sometimes was correctly null would crash it out. If the debugger isn't attached and we just have a crash dump we can't read (happens even on platforms where you're SUPPOSED to be able to read them), then we have to attach the debugger and hope that we can reproduce the obscure state that caused the crash.
Anything that makes debugging statements more robust and less crash-prone is a good thing. End of story. As others have said, if you want something to verify that a pointer is non-null, use an assert().
I said that, too. But sometimes developers don't think to put in every possible assert. Having the C library explode violently when it detects a bug in their code even when they forgot to include an assert which checks for that bug will help to find bugs under such circumstances.
Environments that do not have the hardware-assisted detection for null dereferences can still be targetted by a compiler which generates code which does the check on pointer dereference. That can be included in your debug builds, or even left in production builds if the performance impact is acceptable. I seem remember that some compilers for the Intel 8088 MS-DOS environment had a code generation option for trapping nulls.
Will help to crash your code when you use printf, which (in game development in particular) is 99% for debugging statements.
No, having the C library explode violently at any opportunity does not always help detect bugs, and can be 100% harmful. You're overgeneralizing your own bug circumstances and development environment.
That said, if it matters that much to you, wrap printf with a modified version that asserts on NULL-ish pointer dereferences. Don't attempt to roll back the clock on library development for everyone.
$ echo 'int main(void){ return *(int*)0; }' > crash.c
$ gcc -Wall -Wextra -std=c1x crash.c -o crash
$ ./crash
Segmentation fault
$ ulimit -c unlimited
$ ./crash
Segmentation fault (core dumped) fprintf(stderr, "test %s\n", s != NULL ? s : "NULL");
if(s == NULL) abort();How do we know it's a security hole? Maybe some structure has a "char * " name field, and null indicates "anonymous/has no name". It's an ISO C conformance bug to try to print that with %s, but that's a portability issue. If that's the only problem in the program (everywhere else it checks for null), there is no real issue.
POSIX and C libraries are not entirely about allowing null pointers or not. For instance gettimeofday lets you pass a null pointer for the "struct timezone * " second argument, which indicates that you aren't interested in specifying or retrieving it. On the other hand, a null argument is undefined behavior in getgroups, even if the array size is specified as zero (meaning, return me the size of the array actually needed and I will call you again soon with such an array)! A non-null pointer must be passed as a POSIX-conforming eggcorn. According to the logic of this article, if I pass null I'm committing a security hole, and an OS which allows getgroups(0, NULL) is misguidedly sweeping it under the rug.
Null pointers are not some "bogey man". I built a Lisp dialect in whose implementation a C null pointer represents the nil symbol (empty list, false, ...). It prints as "nil", satsifies the symbolp function and so fort. Lexical variables are initialized to nil if not given an initial expression. Some C library printing a null pointer as (null) seems like nothing by comparison.
Be that all as it may, you cannot actually fix this perceived issue by changing from (null) to crash, cold turkey. Unknown numbers of nonportable programs out there (otherwise well-tested programs) which rely on this will just crash.
I just did a "grep -r -F '(null)' /var/log" on a system here and found a few hits. Most of those are probably via the %s conversion, and all that code would have to be fixed if printf were to assert this. And that's just instances confined to logging, not all output everywhere.
Of course not. Every function which has undefined behaviour upon being passed NULL should blow up on NULL.
In general, anything which can crash should crash, because that's the fastest way to make sure that bugs get found.
printf("%p\n", (void *) 0); // not undefined
It's the specific conversion specifier %s which doesn't have defined behavior.> anything which can crash should crash
__asm__ __volatile__ can crash. It's undefined behavior.
(E.g. the assertion for non-NULL-ity would go on the branch of printf that handles %s.)
I'll take 'uses of Option/Maybe' for 200 Alex
Some might say it's overkill, but if you can have a char* be null, then for most intents and purposes you have two states to deal with on any usage of that value.
Just because such a design can exist doesn't mean we should hobble our tools to support it.
In Lisp, CAR and CDR accessors take a cons cell or NIL, which isn't a cons cell, and has no such fields.
It wasn't always like that; according to the HOPL paper on evolution of Lisp, InterLisp introduced this nicety and it was copied by others.
These functions are complicated with that extra state, but programs are simplified.
A null "char *" indicating "this object has no name" is better than some special string value doing the same, because it's different from every possible string value.
This is functionally equivalent, but forces existence checks when they're needed. And by removing "null" from your vocabulary, an entire class of errors disappears.
With C this is a bit trickier to ensure. But I think the design pattern can still be useful. And it doesn't make code more complicated, but rather it makes the complicated aspects explicit . Instead of testing that name == null, you test that name.exists is False, and if not you use name.value. And you make null a banned word in your codebase.
The result is that you _never_ have to ask "is this char* possibly null?". The answer is always "no". So you remove superfluous checks
I think this pattern works much better when you have stronger type checking, though...
At least, presuming a non-nullable char pointer is a perfectly sensible implementation of String.
But it could just as well be that only some of your strings are nullable, and so you could get rid of certain code paths.
Incidentally, while Maybe String can be a nullable char pointer, Maybe (Maybe String) needs a different representation. This is where "nullable" in C# breaks down compared to a real option type - but only because they conflate type and representation.
That's why SQL NULL is also a bad idea.
MariaDB [(none)]> select null as a union all
select 1 as a;
+------+
| a |
+------+
| NULL |
| 1 |
+------+
2 rows in set (0.01 sec)
MariaDB [(none)]> select * from (select null as a
union all select 1 as a) T where a >= 1;
+------+
| a |
+------+
| 1 |
+------+
1 row in set (0.00 sec)
MariaDB [(none)]> select * from (select null as a
union all select 1 as a) T where NOT a >= 1;
Empty set (0.00 sec)SELECT SUM(x) FROM foo;
If some x values are NULL and others not, you will never know.
https://www.gnu.org/software/libc/manual/html_node/Other-Out...
It's documented.
14 EFAULT Bad address. The system detected an invalid address in
attempting to use an argument of a call.
Is that an example of a misguided operating system? The kernel should kill the offending process instead of sweeping the bug under the rug with an errno code that might not be examined.Those who live in glass houses ...
It is better to follow the standard in such cases, so that portable code written against the standard runs nicely.
However (claims the OP) in the case of printf, the %s null pointer case is explicitly defined to be undefined behavior and hence the implementation should handle this either implicitly (by crashing with null pointer dereference) or explicitly by calling abort(), as that will teach programmers not to rely on superficially benign implementations of undefined behavior.
And ISO and IEEE standards can't be "misguided", just competing operating systems that are more successful.
> explicitly defined to be undefined behavior
Contradiction.
Undefined behavior is not defined, explicitly or otherwise. There can be explicit wording that some requirement is not being made.
When ISO C explicitly states that there is an absence of a requirement, that does not carry any more or less weight then when behavior is undefined by omission of any wording of a requirement. All UB is equal.
> not to rely on superficially benign implementations of undefined behavior.
An excellent example of beneficially fixing undefined behavior is to implement strict left to right ordering on all operations and their side effects.
A language or language dialect in which evaluation order is defined is safer and better for engineering.
That's not a claim that I would ever make. But the right time to bring up objections is during the drafting of the standard.
If you aim to implement a standard you should have a very good reason before you implement some interface differently than specified. I would argue that these return values are not a good enough reason. gets(3) on the other hand would hardly be missed...
> Undefined behavior is not defined, explicitly or otherwise. There can be explicit wording that some requirement is not being made.
In C, undefined behavior has a quite specific meaning: invoking undefined behavior is an error in the program and the implementation is at liberty to do anything it pleases.
"undefined behavior: behavior upon use of a nonportable or erroneous program construct or of erroneous data, for which this International Standard imposes no requirements."
Your example of evaluation order is not undefined behavior, but unspecified behavior: there is a set of possible orderings and the implementation may pick any one of them.
That doesn't match my understanding, which is that undefined behavior is about erroneous constructs or data, or non-portable constructs.
Oh look, you quoted it. Thanks!
Some undefined behaviors are important sources for useful, documented extensions.
printf gets used all the time in systems code. I count 1221 occurrences of printf (and friends) in BIND 9 alone.
More to the point, "(null)" was just an example of a debugging-hostile pro-vulnerability attempt to silently "fix" bad data.
The software running in your car, plane, spacecraft and the likes have very strict regulations and you will find it radically different from systems code found in a linux distribution, even if written in the same language!
See http://lars-lab.jpl.nasa.gov/JPL_Coding_Standard_C.pdf for more details. I wouldn't want to write code like this as it sounds very unproductive, but their reliability and astronomically small number of bugs speaks for itself.
I can't remember the last time my DNS didn't work. The internet can afford to be 'good enough', life critical software can't.