linux-kernel - Re: [RFC v2 4/4] backports: add kernl integration support to gentree.py

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1414741851.3014.13.camel@jlt4.sipsolutions.net>
Date:	Fri, 31 Oct 2014 08:50:51 +0100
From:	Johannes Berg <johannes@...solutions.net>
To:	"Luis R. Rodriguez" <mcgrof@...e.com>
Cc:	Stefan Assmann <sassmann@...nic.de>,
	"Luis R. Rodriguez" <mcgrof@...not-panic.com>,
	backports@...r.kernel.org, linux-kernel@...r.kernel.org,
	yann.morin.1998@...e.fr, mmarek@...e.cz
Subject: Re: [RFC v2 4/4] backports: add kernl integration support to
 gentree.py

On Wed, 2014-10-29 at 17:00 +0100, Luis R. Rodriguez wrote:

>  backport/Kconfig                              |  54 --------
>  backport/Kconfig.package                      |  31 +++++
>  backport/Kconfig.sources                      |  23 ++++

I think you should do this split as a separate patch.

> +# these will be generated
> +source "$BACKPORT_DIR/Kconfig.kernel"
> +source "$BACKPORT_DIR/Kconfig.versions"
> +source "$BACKPORT_DIR/Kconfig.sources"

Not true - Kconfig.sources exists below?

> +ifneq ($(BACKPORT_PACKAGE),)

I think it'd be easier to understand if you called this

BACKPORT_INTEGRATED

and inverted the logic.

> +        # Only the kconfig 'mainmenu' entries grok variables, we want to keep
> +        # this the main Kconfig as part of our program to be able to give it
> +        # enough dynamic information
> +        k = open(os.path.join(args.outdir, 'Kconfig'), 'w')
> +        k.write('config BACKPORT_DIR\n')
> +        k.write('\tstring\n')
> +        k.write('\tdefault "backports/"\n')
> +        k.write('config BACKPORTS_VERSION\n')
> +        k.write('\tstring\n')
> +        k.write('\tdefault "%s"\n' % backports_version)
> +        k.write('config BACKPORTED_KERNEL_VERSION\n')
> +        k.write('\tstring\n')
> +        k.write('\tdefault "%s"\n' % kernel_version)
> +        k.write('config BACKPORTED_KERNEL_NAME\n')
> +        k.write('\tstring\n')
> +        k.write('\tdefault "%s"\n' % args.base_name)
> +        k.write('\n')
> +        k.write('menuconfig BACKPORT_LINUX\n')
> +        k.write('\tbool "Backport %s %s (backports %s)"\n' % (args.base_name, kernel_version, backports_version))
> +        k.write('\tdefault n\n')
> +        k.write('\t---help--- \n')
> +        k.write('\t  Enabling this will let give you the opportunity to use\n')
> +        k.write('\t  features and drivers backported from %s %s\n' % (args.base_name, kernel_version))
> +        k.write('\t  on the kernel your are using. This is experimental and you should\n')
> +        k.write('\t  say no unless you\'d like to help test things.\n')
> +        k.write('\n')
> +        k.write('if BACKPORT_LINUX\n')
> +        k.write('\n')
> +        k.write('source "$BACKPORT_DIR/Kconfig.versions"\n')
> +        k.write('source "$BACKPORT_DIR/Kconfig.sources"\n')
> +        k.write('\n')
> +        k.write('endif # BACKPORT_LINUX\n')

This is really really ugly - please just have a file template, and maybe
replace some things in it or provide them as env variables through the
Makefile or so.

> +    if args.integrate:
> +        f = open(os.path.join(args.outdir, '../arch/x86/Kconfig'), 'a')
> +        f.write('source "backports/Kconfig"\n')

That seems like a bad idea - maybe better integrate it under some
arch-independent file?

> +        f.close()
> +        git_debug_snapshot(args, "hooked backport Kconfig to supported architectures")
> +
> +        f = open(os.path.join(args.outdir, '../MAINTAINERS'), 'a')
> +        f.write('M:\tHauke Mehrtens <hauke@...ke-m.de>\n')
> +        f.write('M:\tLuis R. Rodriguez <mcgrof@...not-panic.com>\n')
> +        f.write('L:\tbackports@...r.kernel.org\n')
> +        f.write('T:\tgit://git.kernel.org/pub/scm/linux/kernel/git/backports/backports.git\n')
> +        f.write('F:\tbackports/\n')
> +        f.close()

That's ... odd? doesn't even fit the MAINTAINERS file style? Anyway I
think it's probably not necessary.

> +        git_debug_snapshot(args, "add backport maintainers entry")
> +
> +        f = open(os.path.join(args.outdir, '../Makefile'), 'a')
> +        f.write('ifeq ($(KBUILD_EXTMOD),)\n')
> +        f.write('backports-y := backports/\n')
> +        f.write('vmlinux-dirs += $(patsubst %/,%,$(filter %/, $(backports-y) $(backports-m)))\n')
> +        f.write('vmlinux-alldirs += $(patsubst %/,%,$(filter %/, $(backports-n) $(backports-)))\n')
> +        f.write('backports-y       := $(patsubst %/, %/built-in.o, $(backports-y))\n')
> +        f.write('KBUILD_VMLINUX_MAIN += $(backports-y)\n')
> +        f.write('endif\n')
> +        f.close()

That could be a template file again that you just copy.

[... I need more time to review, don't have much this morning ...]

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/