[pacman-dev] [PATCH] Avoid depending on side effects in assert(...) expressions
andrew.gregory.8 at gmail.com
Thu May 14 02:54:44 UTC 2020
On 05/13/20 at 12:08pm, Anatol Pomozov wrote:
> > ---
> > It's perhaps worth mentioning that nowhere else in the ALPM codebase
> > do we use assert().
> I quite like the idea of defensive programming. This is something that
> I learnt the hard way when I was working with chips firmware.
> So I often add additional checks across the codebase and it saves me
> time during active phase of development/refactoring.
> A bit of context for this assert(). Both "progress" and "complete"
> events should always have a corresponding "init" event where
> progressbar structure is initialized.
> If callback.c received a "progress" event for a non-existent
> progressbar then it is a programming error.
The problem is not defensive programming, it's that assert gets
compiled out under -DNDEBUG, so all your defenses disappear. You say
that the only way there would not be a corresponding progress bar is
if the progress event is called without an init event; that's not
true. If there's a memory allocation failure in the init event there
won't be a progress object. At that point bar and index are
indeterminate and the best we can hope for is a quick segfault. The
callback api should be fixed so that callbacks can indicate an error
that should cancel the download.
I'd also say we should be passing the full list of download items to
the callback so that it's possible to write at least a very simple
multi-download callback without having to maintain a bunch of
complicated state information.
More information about the pacman-dev