On 9/13/23 5:27 AM, Ulrich Mueller wrote:
>>>>>> On Wed, 13 Sep 2023, Eli Schwartz wrote:
>
>>> "|| die" should also be added for the cat command.
>
>> Redirecting output to a file in a directory you have just guaranteed
>> to exist cannot fail.
>
> That's a rather bold statement. I can imagine a number of possible
> failures, e.g. no space left on device, quota exceeded, or a low-level
> I/O error of the filesystem. Also fork or exec of the cat command could
> fail (e.g. out of memory).
>
> While either of these may be unlikely here, best practice is to check
> for errors of _all_ external commands.
The implementation of `die` performs a fork+exec of the sed command,
invokes eerror (many times!) which forks to pipe, invokes $() which
forks, and could behave abnormally due to out of memory. It is not safe
to treat `|| die` as error recovery for an out of memory condition.
The implementation of `die` performs multiple writes to files inside of
${PORTAGE_BUILDDIR}, and could behave abnormally due to write failures.
It is not safe to treat `|| die` as error recovery for a write failure
of the ${PORTAGE_BUILDDIR} drive (whatever the reason for that write
failure).
Regarding:
- no space left on device
- quota exceeded
- low-level I/O error of the filesystem
this isn't "a number of possible failures" :) it is the same failure but
elicited by different prompts. Regarding this, I have already commented...
(And the `|| die` is added to the next revision of this patch either way.)
--
Eli Schwartz
