Parse, Don’t Validate – Some C Safety Tips

Because an error is not an email?

> The whole "prevent double free" claim is completely bogus.

"Completely" means "for all". Are you seriously claiming that "for all instances of double-free, setting the pointer to NULL after freeing it would not help"?

Firstly, `parsing` is just a way to say "serialise from a string". The reverse operation can be done for every type you are creating. If the reverse operation (serialise to a string) does not exist in the interface then adding it gives you a single place to catch all the bugs.

I'm thinking of that recent git bug that occurred because the round-trip of `string -> type -> string` had an error (stripping out the CR character). Using a specific type for a value that is being round-tripped means that a bugfix needs to only be made in the parser function. Storing the value as simple strings would result in needing to put your fix everywhere.

> The trouble I have with this approach (which, conceptually, I agree with) is that it's damned hard to do anything with the parse results.

You're right - it is damn hard, but that is on purpose; if you're doing something with the email that boils down to "treat it like a `char *`" then the potential for error is large.

If you're forced to add in a new use-case to the `email_t` interface then you have reduced the space of potential errors.

For example:

> Want to print that email_t? Then you're right back to char, unless you somehow write your own I/O system that knows about your opaque conventions.

is a bug waiting to surface, because it's an email, not a string, and if you decide to print an email* that was read as a `char *` you might not get what you expect.

It's all a trade-off - if you want more flexibility with the value stored in a variable, then sure, you can have it but it comes at a cost: some code somewhere almost certainly will eventually use that flexibility to mismatch the type!

If you want to prevent type mismatches, then a lot of flexibility goes out the window.

I'm with you, don't do crap like that. Always return a valid object.

  email_t theEmail = parseEmail(untrustedInput);
  if (theEmail.error != PARSE_OK) {
    return error;
  }

It's the parse-versus-validate hill in this case.

This pseudocode is "Validate" for at least 3 reasons:

Forgetting to check:

  this check is fragile: it’s extremely easy to forget. Because its return value is unused, it can always be omitted, and the code that needs it would still typecheck.

Repeatable/redundant checks:

  First, it’s just annoying. We already checked that the list is non-empty, why do we have to clutter our code with another redundant check?

  Second, it has a potential performance cost. Although the cost of the redundant check is trivial in this particular example, one could imagine a more complex scenario where the redundant checks could add up, such as if they were happening in a tight loop.

Not using the type system:

  Use a data structure that makes illegal states unrepresentable. Model your data using the most precise data structure you reasonably can. If ruling out a particular possibility is too hard using the encoding you are currently using, consider alternate encodings that can express the property you care about more easily. Don’t be afraid to refactor.

> How do you know PARSE_ERROR is not email_t

It has to be for it to compile, right? Which means that email_t is the type which represents both valid and invalid emails. How do you know if it's valid? You remember to write a check for it. Why not just save yourself some keystrokes and use char* instead. This is validate, not parse.