kern/intr: have intr_event_destroy() return success on NULL event

[ehem_freebsd_m5p.com]

Summary

A NULL event likely indicates the event was already released. In such
case success is more useful than failure, so return 0. This follows the
pattern of gracefully handling NULL events.

[ehem_freebsd_m5p.com]

Trying for other reviewers since this has already been sitting around for months with no action.

[ehem_freebsd_m5p.com]

"My kingdom for a review"?

The real issue is if a caller actually checks the return code, returning an error would cause the caller to go into error recovery mode. This could be retry the destroy later, or might very well be a panic(). This is more useful to tell the caller to proceed with release/destroy action, not go into error recovery.

[markj]

I think this is fairly arbitrary. intr_event_destroy() does not get called frequently, unlike, say, free(). So it's reasonable to be less permissive, since intr_event_destroy(NULL) is likely the result of a programming mistake.

I made an argument along the same lines in D38599. As a matter of principle, I prefer code like this to fail as early as possible, so I don't really agree with the change. That's not an absolute position, but so far there is no example code to make me reconsider.

[ehem_freebsd_m5p.com]

A handy example is D38599. It could test isrc->isrc_event before calling intr_event_destroy(), but if intr_event_destroy() will gracefully handle ie == NULL then it is easier not to bother. If you prefer to have a panic() sooner rather than later, the

if (ie == NULL)
        return (EINVAL);

should be taken out and hopefully a panic() will ensue on line 539. If intr_event_destroy() is going to check for ie == NULL then it should do something useful if that occurs. Returning EINVAL is almost certain not to be useful and thus returning 0 seems more sensible.