Re: [PATCH 2/5] modpost: refactor error handling and clarify error/fatal difference

[Quentin Perret <qperret@google.com>]

On Tuesday 01 Dec 2020 at 19:34:15 (+0900), Masahiro Yamada wrote:
> We have 3 log functions. fatal() is special because it lets modpost bail
> out immediately. The difference between warn() and error() is the only
> prefix parts ("WARNING:" vs "ERROR:").
> 
> The intended usage of error() is probably to propagate the return code
> from the function to the exit code of modpost, as check_exports() etc.
> already does. This is a good manner because we should display as many
> issues as possible in a single run of modpost.
> 
> What is annoying about fatal() is that it kills modpost at the first
> error. People would need to run Kbuild again and again until they fix
> all errors.
> 
> But, unfortunately, people tend to do:
> "This case should not be allowed. Let's replace warn() with fatal()."

Indeed :-)

> One of the reasons is probably it is tedious to manually carry the error
> code back to the main() function.

And yes, that was the reason.

> This commit refactors error() so any single call for it automatically
> makes modpost return the error code.
> 
> I also added comments in modpost.h for warn(), error(), and fatal().
> 
> Again, please use fatal() only when you have a strong reason to do so.
> For example:
> 
>   - Memory shortage (i.e. malloc() etc. has failed)
>   - The ELF file is broken, and there is no point to continue parsing
>   - Something really odd has happened
> 
> For general coding errors, please use error().
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

I gave it a go and the error is propagated correctly, so FWIW:

Tested-by: Quentin Perret <qperret@google.com>

Thanks,
Quentin