Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

[Joel Fernandes]

On Thu, Mar 07, 2019 at 01:59:12PM +0900, Masahiro Yamada wrote:
[snip]
> > > [2]
> > >
> > > The shell script keeps running
> > > even when an error occurs.
> > >
> > > If any line in a shell script fails,
> > > probably it went already wrong.
> > >
> > > I highly recommend to add 'set -e'
> > > at the very beginning of your shell script.
> > >
> > > It will propagate the error to Make.
> >
> > Ok I will consider making it -e. But please note that, the script itself does
> > not have any bugs.
> 
> Heh.
> I do not know why you are so confident with your code, though.
> 
> OK, let's say this script has no bug.
> 
> But, the script may fail for a different reason.
> For example, what if cpio is not installed on the user's machine.
> 
> In such a situation, this script will produce empty
> kernel/kheaders_data.tar.xz, but the build system
> will continue running, and succeed.
> 
> Don't you think it will better to let the build immediately fail
> than letting the user debug a wrongly created image ?

Yes, I do think it is better and I am doing that in the next revision
already. It is appended below.

> > > [5]
> > > strip-comments.pl is short enough,
> > > and I do not assume any other user.
> > >
> > >
> > > IMHO, it would be cleaner to embed the one-liner perl
> > > into the shell script, for example, like follows:
> > >
> > > find  $cpio_dir -type f -print0 |
> > >  xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;'
> >
> > This does not work. I already tried it. But I guess I could generate the
> > script on the fly.
> 
> 
> I tested my code, and it worked for me.
> 
> perl -e <command_line>
> 
> is useful to run short code directly.
> 

Sorry, previous attempts to use perl -pie didn't work but I tried your one
liner and it does work. Below is the updated patch with this and all the
other nits addressed including the module.lds one.

About module.lds for ia64 architecture, sorry to miss that - I did not test
all architectures, just arm64 and x86 which I have hardware for.
Interestingly kbuild robot didn't catch it either. Anyway it is addressed below.

Thanks a lot for all the great review.

---8<-----------------------