tech-kern archive

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

Re: config: makeoptions support for conditions



Thanks for the comments. I hope this attempt is better.

I chose to make expr_eval and expr_free non-static, but not fixsel.
selectopt in mkmakefile.c is just a copy of fixsel.
I don't know if that is best.

I also set both nv_str and nv_ptr, I don't know if that is
consistent with the intended use of nvlist.

Quentin Garnier wrote:
On Thu, Mar 12, 2009 at 02:19:48PM +0200, Yorick Hardy wrote:
config(5) says:

 Conditions
     Finally, source file selection is possible through the help of condition-
     als, referred to as condition later in this documentation.  The syntax
     for those conditions uses well-known operators ( ``&'', ``|'' and ``!'')
     to combine options and attributes.

and
     makeoptions condition name+=value [, condition name+=value]
     Appends to a definition in the generated Makefile.

However, makeoptions does not accept a general condition.
Is the attached patch acceptable?

Not really, sorry.

[...]
Index: usr.bin/config/gram.y
===================================================================
RCS file: /cvsroot/src/usr.bin/config/gram.y,v
retrieving revision 1.18
diff -u -r1.18 gram.y
--- usr.bin/config/gram.y       28 Dec 2008 01:23:46 -0000      1.18
+++ usr.bin/config/gram.y       12 Mar 2009 11:57:22 -0000
@@ -507,7 +507,8 @@
        condmkoption;
condmkoption:
-       WORD mkvarname PLUSEQ value     { appendcondmkoption($1, $2, $4); };
+       WORD mkvarname PLUSEQ value     { appendcondmkoption($1, $2, $4); } |
+       fexpr mkvarname PLUSEQ value    { appendcondmkoptexpr($1, $2, $4); };

Nasty hack 1.  There's no reason to treat a single condition differently
from a full expression.

[...]
+appendcondmkoptexpr(struct nvlist *l, const char *name, const char *value)
+{
+       struct nvlist *nv;
+
+       nv = newnv(name, value, l, 0, NULL);
+       *nextappmkopt = nv;
+       nextappmkopt = &nv->nv_next;

Nasty hack 2.  Abusing the list of unconditional make options to squeeze
in conditional options...

It's actually much simpler to only use 'fexpr' in the grammar and
evaluate it in print_condmkoptions.  You don't have to be smart in how
you fill in condmkopttab, it can actually just be a nvlist.

I guess the reason it was done that way was to minimize the number of
evaluations of options, but there's no point in doing that if it uses
expressions.

I thought I had started a patch for that when Matt asked me about it,
but I can't find it anymore...



--
Kind regards,

Yorick Hardy
Index: usr.bin/config/defs.h
===================================================================
RCS file: /cvsroot/src/usr.bin/config/defs.h,v
retrieving revision 1.28
diff -u -r1.28 defs.h
--- usr.bin/config/defs.h       15 Feb 2009 01:39:54 -0000      1.28
+++ usr.bin/config/defs.h       13 Mar 2009 07:08:56 -0000
@@ -402,7 +402,7 @@
 struct nvlist *fsoptions;      /* filesystems */
 struct nvlist *mkoptions;      /* makeoptions */
 struct nvlist *appmkoptions;   /* appending mkoptions */
-struct hashtab *condmkopttab;  /* conditional makeoption table */
+struct nvlist *condmkopttab;   /* conditional makeoption table */
 struct hashtab *devbasetab;    /* devbase lookup */
 struct hashtab *devroottab;    /* attach at root lookup */
 struct hashtab *devatab;       /* devbase attachment lookup */
@@ -466,6 +466,8 @@
 int    fixdevsw(void);
 void   addfile(const char *, struct nvlist *, int, const char *);
 void   addobject(const char *, struct nvlist *, int);
+int    expr_eval(struct nvlist *, int (*)(const char *, void *), void *);
+void   expr_free(struct nvlist *);
 
 /* hash.c */
 struct hashtab *ht_new(void);
@@ -490,7 +492,7 @@
 void   addfsoption(const char *);
 void   addmkoption(const char *, const char *);
 void   appendmkoption(const char *, const char *);
-void   appendcondmkoption(const char *, const char *, const char *);
+void   appendcondmkoption(struct nvlist *, const char *, const char *);
 void   deffilesystem(const char *, struct nvlist *, struct nvlist *);
 void   defoption(const char *, struct nvlist *, struct nvlist *);
 void   defflag(const char *, struct nvlist *, struct nvlist *, int);
Index: usr.bin/config/files.c
===================================================================
RCS file: /cvsroot/src/usr.bin/config/files.c,v
retrieving revision 1.9
diff -u -r1.9 files.c
--- usr.bin/config/files.c      20 Jan 2009 18:20:48 -0000      1.9
+++ usr.bin/config/files.c      13 Mar 2009 07:08:57 -0000
@@ -69,9 +69,6 @@
 static int     fixcount(const char *, void *);
 static int     fixfsel(const char *, void *);
 static int     fixsel(const char *, void *);
-static int     expr_eval(struct nvlist *,
-                   int (*)(const char *, void *), void *);
-static void    expr_free(struct nvlist *);
 
 void
 initfiles(void)
@@ -496,7 +493,7 @@
  * No short circuiting ever occurs.  fn must return 0 or 1 (otherwise
  * our mixing of C's bitwise & boolean here may give surprises).
  */
-static int
+int
 expr_eval(struct nvlist *expr, int (*fn)(const char *, void *), void *context)
 {
        int lhs, rhs;
@@ -527,7 +524,7 @@
 /*
  * Free an expression tree.
  */
-static void
+void
 expr_free(struct nvlist *expr)
 {
        struct nvlist *rhs;
Index: usr.bin/config/gram.y
===================================================================
RCS file: /cvsroot/src/usr.bin/config/gram.y,v
retrieving revision 1.18
diff -u -r1.18 gram.y
--- usr.bin/config/gram.y       28 Dec 2008 01:23:46 -0000      1.18
+++ usr.bin/config/gram.y       13 Mar 2009 07:08:59 -0000
@@ -507,7 +507,7 @@
        condmkoption;
 
 condmkoption:
-       WORD mkvarname PLUSEQ value     { appendcondmkoption($1, $2, $4); };
+       fexpr mkvarname PLUSEQ value    { appendcondmkoption($1, $2, $4); };
 
 no_mkopt_list:
        no_mkopt_list ',' no_mkoption |
Index: usr.bin/config/main.c
===================================================================
RCS file: /cvsroot/src/usr.bin/config/main.c,v
retrieving revision 1.34
diff -u -r1.34 main.c
--- usr.bin/config/main.c       14 Feb 2009 21:28:58 -0000      1.34
+++ usr.bin/config/main.c       13 Mar 2009 07:09:01 -0000
@@ -95,6 +95,7 @@
 static struct nvlist **nextopt;
 static struct nvlist **nextmkopt;
 static struct nvlist **nextappmkopt;
+static struct nvlist **nextcndmkopt;
 static struct nvlist **nextfsopt;
 
 static void    usage(void) __dead;
@@ -280,6 +281,7 @@
        nextopt = &options;
        nextmkopt = &mkoptions;
        nextappmkopt = &appmkoptions;
+       nextcndmkopt = &condmkopttab;
        nextfsopt = &fsoptions;
 
        /*
@@ -947,21 +949,13 @@
  * Add a conditional appending "make" option.
  */
 void
-appendcondmkoption(const char *selname, const char *name, const char *value)
+appendcondmkoption(struct nvlist *cnd, const char *name, const char *value)
 {
-       struct nvlist *nv, *lnv;
-       const char *n;
-
-       n = strtolower(selname);
-       nv = newnv(name, value, NULL, 0, NULL);
-       if (ht_insert(condmkopttab, n, nv) == 0)
-               return;
+       struct nvlist *nv;
 
-       if ((lnv = ht_lookup(condmkopttab, n)) == NULL)
-               panic("appendcondmkoption");
-       for (; lnv->nv_next != NULL; lnv = lnv->nv_next)
-               /* search for the last list element */;
-       lnv->nv_next = nv;
+       nv = newnv(name, value, cnd, 0, NULL);
+       *nextcndmkopt = nv;
+       nextcndmkopt = &nv->nv_next;
 }
 
 /*
Index: usr.bin/config/mkmakefile.c
===================================================================
RCS file: /cvsroot/src/usr.bin/config/mkmakefile.c,v
retrieving revision 1.10
diff -u -r1.10 mkmakefile.c
--- usr.bin/config/mkmakefile.c 20 Feb 2009 05:20:25 -0000      1.10
+++ usr.bin/config/mkmakefile.c 13 Mar 2009 07:09:02 -0000
@@ -75,6 +75,7 @@
 static void emitincludes(FILE *);
 static void emitappmkoptions(FILE *);
 static void emitsubs(FILE *, const char *, const char *, int);
+static int  selectopt(const char *, void *);
 
 int
 mkmakefile(void)
@@ -543,21 +544,6 @@
        }
 }
 
-static int
-print_condmkopts(const char *name, void *value, void *arg)
-{
-       struct nvlist *nv;
-       FILE *fp = arg;
-
-       if (ht_lookup(selecttab, name) == 0)
-               return (0);
-
-       for (nv = value; nv != NULL; nv = nv->nv_next)
-               fprintf(fp, "%s+=%s\n", nv->nv_name, nv->nv_str);
-
-       return (0);
-}
-
 /*
  * Emit appending makeoptions.
  */
@@ -569,5 +555,18 @@
        for (nv = appmkoptions; nv != NULL; nv = nv->nv_next)
                fprintf(fp, "%s+=%s\n", nv->nv_name, nv->nv_str);
 
-       ht_enumerate(condmkopttab, print_condmkopts, fp);
+       for (nv = condmkopttab; nv != NULL; nv = nv->nv_next)
+       {
+               if (expr_eval(nv->nv_ptr, selectopt, NULL))
+                       fprintf(fp, "%s+=%s\n", nv->nv_name, nv->nv_str);
+               expr_free(nv->nv_ptr);
+       }
+}
+
+static int
+/*ARGSUSED*/
+selectopt(const char *name, void *context)
+{
+
+       return (ht_lookup(selecttab, name) != NULL);
 }


Home | Main Index | Thread Index | Old Index