Doubts about paranoic defensive programming style about neomutt HOT 3 CLOSED

It's a tricky situation...

Ultimately, I'm running a project with few users.
If something doesn't quite work right, or outright fails, a user might raise an issue.
If NeoMutt crashes, or an assertion fails, the user is more likely to abandon NeoMutt altogether.

This makes me lean towards defensive programming.

Yes, realloc() and friends are a mess, but there are some things we could still do...

• Perhaps we could flag functions with __attribute__((nonnull)) (as a stepping stone)
  I think Coverity can leverage this sort of thing.

• There may still be a few custom list implementations that should use TAILQ

• There are LOTS of arrays that would be safer using ARRAY (gahr's baby).
  Reduce the amount of memory handling to a few "safe" places.

[Aside] - If you want to see a real mess, look at the file-opening code.

from neomutt.

It's a tricky situation...

Ultimately, I'm running a project with few users. If something doesn't quite work right, or outright fails, a user might raise an issue. If NeoMutt crashes, or an assertion fails, the user is more likely to abandon NeoMutt altogether.

This makes me lean towards defensive programming.

Hmmm, I understand.

Yes, realloc() and friends are a mess, but there are some things we could still do...

Well, as long as it works, since it's supposedly dead code, I guess we can keep it dead around; I don't mind too much.

* Perhaps we could flag functions with `__attribute__((nonnull))` (as a stepping stone)
  I think Coverity can leverage this sort of thing.

I'm poking the LLVM people to improve their _Nullable qualifier, which I think is better than [[gnu::nonnull]]. However, it's still not in a usable state. Let's see if we can use that in a few years from now.

* There may still be a few custom list implementations that should use `TAILQ`

* There are LOTS of arrays that would be safer using `ARRAY` (gahr's baby).
  Reduce the amount of memory handling to a few "safe" places.

[Aside] - If you want to see a real mess, look at the file-opening code.

:-)

Thanks!

from neomutt.

this is exactly the type of thing assert() is for, and static analyzers can make good use of it. attribute((nonnull)) is a specialized variation on that which is also visible outside the function.

just don't use it on unsanitized user-/system-provided data.

from neomutt.