> for most programs, a disadvantage of the particular way that stralloc is implemented in qmail is that you have to check every single copy or concatenation operation for an out-of-memory error, as you see above. this makes your code a lot longer. many applications are better off just aborting inside the memory allocation function if they run out of memory; getting out-of-memory handling correct is very difficult, especially if you don't devote a massive amount of effort to testing out-of-memory conditions (because they won't occur often enough just by chance to test your error-handling path)
You could do that. Or you could put a field in the struct that stores an error flag. If flag is set, all `stralloc` functions return immediately. When they fail, they set the flag and then return.
This lets you do:
stralloc_copys(&rcptto,""));
stralloc_copys(&mailfrom,addr.s));
stralloc_0(&mailfrom));
if (stralloc_error(rcptto) || stralloc_error(mailfrom)) {
die_nomem();
}
I'd go one further and make the error checker function take variable arguments, so that the last line looks like this:
if (stralloc_error (rcptto, mailfrom, NULL)) {
die_nomem();
}
IME, forgetting to terminate the parameter list with a NULL almost always causes the program to blow up on the very first execution.
> but please don't fill your program with fifty zillion string buffers of arbitrarily chosen sizes and then try to separately pass the right size in seventy zillion string-processing function calls. your code will be hard to read, buggy, and probably insecure. factor string buffer length handling into a small part of your program so that most of your code never has to think about string buffer lengths
I agree, but after years and years of looking at and writing idiomatic safe C code, I am now of the opinion that a string library is, while a better approach to slinging around raw strings, still very much the wrong approach.
Nothing stops the developer from doing Parse, Don't Validate! in C, and this means that seeing C strings being used anywhere other than at the boundaries to the system evokes my code-smell senses.
these are very good ideas; thank you! by coincidence yesterday i was looking at some code i wrote in golang six years ago which uses this same approach to error handling for i/o errors. i wonder if you might be better off putting the error flag in the allocator rather than the individual string objects?
i do think parse, don't validate is much more difficult in c; c's type system is not strong enough to give you the kinds of soundness guarantees you get from ocaml or haskell. if you forget a type case in a switch, there's not a whole lot you can do to get the compiler to complain about it
the code i quoted above is from qmail-smtpd.c, which is 373 lines of code like the above and contains all of qmail's smtp input logic except for ip_scanbracket, which parses strings like [127.0.0.1] and is shared with dns.c. it's not clear to me that a parse, don't validate approach would consist of much more than just the parser
maybe using a parser generator for all your input and output handling would help? i'm still skeptical that something like a text editor or a macro processor is going to have a large body of code that is free of string handling
> i do think parse, don't validate is much more difficult in c; c's type system is not strong enough to give you the kinds of soundness guarantees you get from ocaml or haskell. if you forget a type case in a switch, there's not a whole lot you can do to get the compiler to complain about it
Sure, I agree, but we're talking about strings here. Instead of a function taking or returning an email address in a generic string type, it can take or return an email address type.
For example, construction of a value of type `email_t` can take a parameter of raw string. Then any function in the rest of the code that receives an email would receive an `email_t`, not a `char ` or some other generic string type.
> it's not clear to me that a parse, don't validate approach would consist of much more than just the parser
It might often be nothing but* a parser, such as `email_t`, but it means that no `str()` function would then be used by the caller - any operation on the `email_t`, if `email_t` is an opaque pointer, would used the `email_type_()` functions, because the users of any `email_t` value cannot access, or even see, the fields inside an `email_t` value.
This means that passing an email to a function expecting a name would cause a compiler error.
For the IP address example you mention, that definitely should be parsed only once into the quad-byte or quad-quad integer fields.
I mean, I'm looking over my previous projects: every single instance of a string I am using is actually not just "generic string"; there's an associated type with it (name, description, comment, whatever). Making those into different types with their own operations means that the compiler will generate errors if I try to use a `description_t` where a `name_t` is expected.
indeed it is the case in qmail that ip_scanbracket populates a struct ip_address. but rcptto, where the destination email address goes, is just a byte buffer in a very simple ad-hoc format which, if i understand correctly, gets written to a pipe; qmail's privilege separation design, which its author to a significant extent came to regret, adds some extra difficulties here by requiring things to run in separate processes
what you read from or write to a pipe is, at that point, necessarily just a generic string. you could write some kind of generic serialization layer, but doing that in c requires a preprocessor, and unmarshaling things in a statically type-safe way really requires compiler support for sum types, which c doesn't have
aside from that, i think it's pretty likely that trying to parse the email address in the qmail-smtpd process would have made the code more bug-prone rather than less so
> is just a byte buffer in a very simple ad-hoc format which, if i understand correctly, gets written to a pipe; qmail's privilege separation design, which its author to a significant extent came to regret, adds some extra difficulties here by requiring things to run in separate processes
> ...
> what you read from or write to a pipe is, at that point, necessarily just a generic string.
Aren't the reader and writer of the pipe part of the same software package?
If they are, then safe[1] functions for that type make sense:
This still means that you're only ever passing around `email_t` values, not `char *` values.
On the other hand, if the reader and writer of the pipe are in different packages, then the pipe is the boundary for each of them, and you wouldn't be passing language native types without first serialising to a language independent representation anyway.
[1] By "safe" I mean that they don't overflow and that the actual binary format allows the `from_bytes` function to determine when the input could be malicious.
yes, they are part of the same software package. i guess qmail-smtpd does have to parse the email address somewhat in order to match the domain against rcpthosts so it can reject attempts to relay mail? and yeah, that's what the addrparse() call does in the code i quoted—but it just stores the email in addr as a canonicalized string. so it may end up rewriting things like lelanthran@[10.1.2.3] as lelanthran@lelanthran.com, for example, and also has code to strip out explicit smtp source routes (@foo.com:lelanthran@lelanthran.com) which were still in theory required when qmail came out
so when i implied that 'trying to parse the email address in the qmail-smtpd process' was not a thing that was already being done, i was wrong, so plausibly your recommendation is in fact applicable; maybe it would have been better to parse the email address into a struct with user and host fields, then have email_to_bytes represent the email <kragen@gentle.dyn.ml.org> as T6:kragen,17:gentle.dyn.ml.org, instead of as Tkragen@gentle.dyn.ml.org\0. i mean you could generate email_to_bytes with a code generator (an idl compiler) instead of writing it
then you wouldn't have to worry about the possibility that you'd accidentally left an @ in one part or the other—unless you did relay the mail over smtp to some other host, in which case you would have to worry about it anyway
You could do that. Or you could put a field in the struct that stores an error flag. If flag is set, all `stralloc` functions return immediately. When they fail, they set the flag and then return.
This lets you do:
I'd go one further and make the error checker function take variable arguments, so that the last line looks like this: IME, forgetting to terminate the parameter list with a NULL almost always causes the program to blow up on the very first execution.> but please don't fill your program with fifty zillion string buffers of arbitrarily chosen sizes and then try to separately pass the right size in seventy zillion string-processing function calls. your code will be hard to read, buggy, and probably insecure. factor string buffer length handling into a small part of your program so that most of your code never has to think about string buffer lengths
I agree, but after years and years of looking at and writing idiomatic safe C code, I am now of the opinion that a string library is, while a better approach to slinging around raw strings, still very much the wrong approach.
Nothing stops the developer from doing Parse, Don't Validate! in C, and this means that seeing C strings being used anywhere other than at the boundaries to the system evokes my code-smell senses.