[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2



>>> On 24.11.15 at 18:51, <cardoe@xxxxxxxxxx> wrote:
> --- /dev/null
> +++ b/xen/include/linux/kconfig.h
> @@ -0,0 +1,54 @@
> +#ifndef __LINUX_KCONFIG_H
> +#define __LINUX_KCONFIG_H

Neither placement in the source tree nor guard variable should say
"Linux".

> --- /dev/null
> +++ b/xen/scripts/Makefile.host
> @@ -0,0 +1,128 @@

We already have ways to build host programs. Do we really need a
second mechanism, the more an abstract one? I'd prefer a more
minimalistic approach (confined to the xen/scripts/kconfig/ subtree,
which imo actually should be xen/tools/kconfig/). In any even I'd
expect, for everything outside of the actual kconfig/ subtree, a
few words in the commit message towards the rationale for including
those pieces. That'll help future cleanup by clarifying what certain
pieces are actually needed for.

> --- /dev/null
> +++ b/xen/scripts/kconfig/.gitignore
> @@ -0,0 +1,22 @@

Does the top level one (perhaps suitably adjusted) not fit the needs?

> --- /dev/null
> +++ b/xen/scripts/kconfig/Makefile.linux
> @@ -0,0 +1,317 @@

This doesn't seem to be referenced anywhere.

> --- /dev/null
> +++ b/xen/scripts/kconfig/POTFILES.in
> @@ -0,0 +1,12 @@

Do we really need this?

> +void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const 
> char *), void *data, int prevtoken)
> +{
> +     if (!e) {
> +             fn(data, NULL, "y");
> +             return;
> +     }
> +
> +     if (expr_compare_type(prevtoken, e->type) > 0)
> +             fn(data, NULL, "(");
> +     switch (e->type) {
> +     case E_SYMBOL:
> +             if (e->left.sym->name)
> +                     fn(data, e->left.sym, e->left.sym->name);
> +             else
> +                     fn(data, NULL, "<choice>");
> +             break;
> +     case E_NOT:
> +             fn(data, NULL, "!");
> +             expr_print(e->left.expr, fn, data, E_NOT);
> +             break;
> +     case E_EQUAL:
> +             if (e->left.sym->name)
> +                     fn(data, e->left.sym, e->left.sym->name);
> +             else
> +                     fn(data, NULL, "<choice>");
> +             fn(data, NULL, "=");
> +             fn(data, e->right.sym, e->right.sym->name);
> +             break;
> +     case E_LEQ:
> +     case E_LTH:
> +             if (e->left.sym->name)
> +                     fn(data, e->left.sym, e->left.sym->name);
> +             else
> +                     fn(data, NULL, "<choice>");
> +             fn(data, NULL, e->type == E_LEQ ? "<=" : "<");
> +             fn(data, e->right.sym, e->right.sym->name);
> +             break;
> +     case E_GEQ:
> +     case E_GTH:
> +             if (e->left.sym->name)
> +                     fn(data, e->left.sym, e->left.sym->name);
> +             else
> +                     fn(data, NULL, "<choice>");
> +             fn(data, NULL, e->type == E_LEQ ? ">=" : ">");

Please eliminate the copy-and-paste mistake here right away (see
Linux commit f6aad2615c).

> --- /dev/null
> +++ b/xen/scripts/kconfig/gconf.c
> @@ -0,0 +1,1521 @@

I think I had asked before, and with us not wanting any user visible
prompts for now I wonder even more: Do we really need [gmnq]conf?
All I think we really need is conf.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.