Re: [PATCH v4 1/2] package.py: strip_execs: Support for .ko modules

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Thu, 2017-08-24 at 16:13 +0200, Tobias Hagelborn wrote:
> * Support stripping of .ko modules verifying file extension and
>   check of content "vermagic="
> * Minor refactoring (removing lint errors)

Please don't do this. If you want to do "lint" cleanups please do it in
a separate patch. I can't decide if some of the cleanup is just that or
there was a real bug you were fixing.

I'm also a little worried about the overhead of mmaping all files and
then doing text searches on them. Could we avoid that unless its a
"*.ko" file?

I'm also not convinced:

-    libdir = os.path.abspath(dstdir + os.sep + libdir)
+    libdir = os.path.abspath(os.path.join(dstdir, libdir.lstrip(os.sep)))

is an improvement...


>                  # It's a file (or hardlink), not a link
>                  # ...but is it ELF, and is it already stripped?
> -                elf_file = isELF(file)
> +                elf_file = is_elf(file)
>                  if elf_file & 1:
>                      if elf_file & 2:
>                          if qa_already_stripped:
> @@ -132,6 +138,7 @@ def strip_execs(pn, dstdir, strip_cmd, libdir,
> base_libdir, qa_already_stripped=
>                          os.unlink(file)
>                          os.link(inodes[s.st_ino], file)
>                      else:
> +                        # break hardlinks so that we do not strip
> the original.
>                          inodes[s.st_ino] = file
>                          # break hardlink

Might as well delete the now unneeded second comment?

Cheers,

Richard