Re: [PATCH] 1 of 2: default ssid to 0

Re: [PATCH] 1 of 2: default ssid to 0

[Reiner Sailer]

> Message: 7
> Date: Sat, 25 Jun 2005 20:39:20 -0500
> From: aq <aquynh@gmail.com>
> Subject: Re: [Xen-devel] [PATCH] 1 of 2: default ssid to 0
> To: Stefan Berger <stefanb@us.ibm.com>
> Cc: xen-devel@lists.xensource.com
> Message-ID: <9cde8bff05062518394eab73cb@mail.gmail.com>
> Content-Type: text/plain; charset=ISO-8859-1
> 
> On 6/25/05, Stefan Berger <stefanb@us.ibm.com> wrote:
> > Hello!
> > 
> > Attached is the patch that changes the default ssid from 0xffffffff to 
0
> > as discussed in previous emails.
> > 
> > Signed-off-by: Reiner Sailer <sailer@us.ibm.com>
> > Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
> > 
> >    Stefan
> > 
> 
> 
> 
> +     for(j=0; j<= CWALL_MAX_SSIDREFS; j++)
> +         if ((0 < j) &&( j <= CWALL_MAX_TYPES))
> +             ssidrefs[j*CWALL_MAX_TYPES + j - 1] = htons(1);
> +
> 
> why dont you run the loop from j=1?


because row j=0 is the new default ssid and has no types set.
We start with row j=1 and set the "diagonal" types to one.


> regards,
> aq

Thanks
Reiner

__________________________________________________________
Reiner Sailer, Research Staff Member, Secure Systems Department
IBM T J Watson Research Ctr, 19 Skyline Drive, Hawthorne NY 10532
Phone: 914 784 6280  (t/l 863)  Fax: 914 784 6205, sailer@us.ibm.com 
http://www.research.ibm.com/people/s/sailer/

Re: [PATCH] 1 of 2: default ssid to 0

[aq]

On 6/26/05, Reiner Sailer <sailer@us.ibm.com> wrote:
> > Message: 7
> > Date: Sat, 25 Jun 2005 20:39:20 -0500
> > From: aq <aquynh@gmail.com>
> > Subject: Re: [Xen-devel] [PATCH] 1 of 2: default ssid to 0
> > To: Stefan Berger <stefanb@us.ibm.com>
> > Cc: xen-devel@lists.xensource.com
> > Message-ID: <9cde8bff05062518394eab73cb@mail.gmail.com>
> > Content-Type: text/plain; charset=ISO-8859-1
> >
> > On 6/25/05, Stefan Berger <stefanb@us.ibm.com> wrote:
> > > Hello!
> > >
> > > Attached is the patch that changes the default ssid from 0xffffffff to
> 0
> > > as discussed in previous emails.
> > >
> > > Signed-off-by: Reiner Sailer <sailer@us.ibm.com>
> > > Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
> > >
> > >    Stefan
> > >
> >
> >
> >
> > +     for(j=0; j<= CWALL_MAX_SSIDREFS; j++)
> > +         if ((0 < j) &&( j <= CWALL_MAX_TYPES))
> > +             ssidrefs[j*CWALL_MAX_TYPES + j - 1] = htons(1);
> > +
> >
> > why dont you run the loop from j=1?
> 
> 
> because row j=0 is the new default ssid and has no types set.
> We start with row j=1 and set the "diagonal" types to one.

i meant in the above loop, if start from j=1 you can eliminate the
check (if 0<j), because in case j=0 you does nothing.

regards,
aq

Re: [PATCH] 1 of 2: default ssid to 0

[David Hopwood]

Reiner Sailer wrote:
>>From: aq <aquynh@gmail.com>

>>+     for(j=0; j<= CWALL_MAX_SSIDREFS; j++)
>>+         if ((0 < j) &&( j <= CWALL_MAX_TYPES))
>>+             ssidrefs[j*CWALL_MAX_TYPES + j - 1] = htons(1);
>>+
>>
>>why dont you run the loop from j=1?
> 
> because row j=0 is the new default ssid and has no types set.
> We start with row j=1 and set the "diagonal" types to one.

I think aq's point was that the above loop is equivalent to:

   #include <stdlib.h> // for 'min'

   for (j = 1; j <= min(CWALL_MAX_SSIDREFS, CWALL_MAX_TYPES); j++) {
       ssidrefs[j*CWALL_MAX_TYPES + j - 1] = htons(1);
   }

which is arguably clearer.

-- 
David Hopwood <david.nospam.hopwood@blueyonder.co.uk>

[PATCH] 1 of 2: default ssid to 0

[Stefan Berger]

[-- Attachment #1: Type: text/plain, Size: 229 bytes --]

Hello!

Attached is the patch that changes the default ssid from 0xffffffff to 0 
as discussed in previous emails. 

Signed-off-by: Reiner Sailer <sailer@us.ibm.com>
Signed-off-by: Stefan Berger <stefanb@us.ibm.com>

   Stefan



[-- Attachment #2: def_ssid_0.diff --]
[-- Type: application/octet-stream, Size: 11110 bytes --]

diff -uprN xeno-unstable.bk.orig/tools/policy/policy_tool.c xeno-unstable.bk/tools/policy/policy_tool.c
--- xeno-unstable.bk.orig/tools/policy/policy_tool.c	2005-06-23 10:20:03.000000000 -0400
+++ xeno-unstable.bk/tools/policy/policy_tool.c	2005-06-25 14:20:02.000000000 -0400
@@ -234,14 +234,14 @@ void acm_dump_policy_buffer(void *buf, i
 /*************************** set policy ****************************/
 
 int acm_domain_set_chwallpolicy(void *bufstart, int buflen) {
-#define CWALL_MAX_SSIDREFS      	5
+#define CWALL_MAX_SSIDREFS      	6
 #define CWALL_MAX_TYPES  		10
 #define CWALL_MAX_CONFLICTSETS		2
 
      struct acm_chwall_policy_buffer *chwall_bin_pol = (struct acm_chwall_policy_buffer *)bufstart;
      domaintype_t *ssidrefs, *conflicts;
      int ret = 0;
-     int i,j;
+     int j;
 
      chwall_bin_pol->chwall_max_types = htons(CWALL_MAX_TYPES);
      chwall_bin_pol->chwall_max_ssidrefs = htons(CWALL_MAX_SSIDREFS);
@@ -261,13 +261,13 @@ int acm_domain_set_chwallpolicy(void *bu
 			   return -1; /* not enough space */
 
      ssidrefs = (domaintype_t *)(bufstart+ntohs(chwall_bin_pol->chwall_ssid_offset));
-     for(i=0; i< CWALL_MAX_SSIDREFS; i++) {
-	     for (j=0; j< CWALL_MAX_TYPES; j++)
-		     ssidrefs[i*CWALL_MAX_TYPES + j] = htons(0);
-	     /* here, set type i for ssidref i; generally, a ssidref can have multiple chwall types */
-	     if (i < CWALL_MAX_SSIDREFS)
-		     ssidrefs[i*CWALL_MAX_TYPES + i] = htons(1);
-     }
+     memset(ssidrefs, 0, CWALL_MAX_TYPES*CWALL_MAX_SSIDREFS*sizeof(domaintype_t));
+
+     /* now set type j-1 for ssidref i+1 */
+     for(j=0; j<= CWALL_MAX_SSIDREFS; j++)
+         if ((0 < j) &&( j <= CWALL_MAX_TYPES))
+             ssidrefs[j*CWALL_MAX_TYPES + j - 1] = htons(1);
+
      ret += CWALL_MAX_TYPES*CWALL_MAX_SSIDREFS*sizeof(domaintype_t);
      if ((buflen - ret) < (CWALL_MAX_CONFLICTSETS*CWALL_MAX_TYPES*sizeof(domaintype_t)))
 			   return -1; /* not enough space */
@@ -276,10 +276,10 @@ int acm_domain_set_chwallpolicy(void *bu
      conflicts = (domaintype_t *)(bufstart + 
 				  ntohs(chwall_bin_pol->chwall_conflict_sets_offset));
      memset((void *)conflicts, 0, CWALL_MAX_CONFLICTSETS*CWALL_MAX_TYPES*sizeof(domaintype_t));
-     /* just 1 conflict set [0]={2,3}, [1]={0,5,6} */
+     /* just 1 conflict set [0]={2,3}, [1]={1,5,6} */
      if (CWALL_MAX_TYPES > 3) {
 	     conflicts[2] = htons(1); conflicts[3] = htons(1); /* {2,3} */
-	     conflicts[CWALL_MAX_TYPES] = htons(1); conflicts[CWALL_MAX_TYPES+5] = htons(1); 
+	     conflicts[CWALL_MAX_TYPES+1] = htons(1); conflicts[CWALL_MAX_TYPES+5] = htons(1); 
 	     conflicts[CWALL_MAX_TYPES+6] = htons(1);/* {0,5,6} */
      }
      ret += sizeof(domaintype_t)*CWALL_MAX_CONFLICTSETS*CWALL_MAX_TYPES;
@@ -287,12 +287,12 @@ int acm_domain_set_chwallpolicy(void *bu
 }
 
 int acm_domain_set_stepolicy(void *bufstart, int buflen) {
-#define STE_MAX_SSIDREFS      	5
-#define STE_MAX_TYPES  		5
+#define STE_MAX_SSIDREFS        6
+#define STE_MAX_TYPES  	        5
 	
     struct acm_ste_policy_buffer *ste_bin_pol = (struct acm_ste_policy_buffer *)bufstart;
     domaintype_t *ssidrefs;
-    int i,j, ret = 0;
+    int j, ret = 0;
 
     ste_bin_pol->ste_max_types = htons(STE_MAX_TYPES);
     ste_bin_pol->ste_max_ssidrefs = htons(STE_MAX_SSIDREFS);
@@ -304,14 +304,14 @@ int acm_domain_set_stepolicy(void *bufst
 	    return -1; /* not enough space */
 
      ssidrefs = (domaintype_t *)(bufstart+ntohs(ste_bin_pol->ste_ssid_offset));
-     for(i=0; i< STE_MAX_SSIDREFS; i++) {
-	     for (j=0; j< STE_MAX_TYPES; j++)
-		     ssidrefs[i*STE_MAX_TYPES + j] = htons(0);
-	     /* set type i in ssidref 0 and ssidref i */
-	     ssidrefs[i] = htons(1); /* ssidref 0 has all types set */
-	     if (i < STE_MAX_SSIDREFS)
-		     ssidrefs[i*STE_MAX_TYPES + i] = htons(1);
-     }
+     memset(ssidrefs, 0, STE_MAX_TYPES*STE_MAX_SSIDREFS*sizeof(domaintype_t));
+     /* all types 1 for ssidref 1 */
+     for(j=0; j< STE_MAX_TYPES; j++)
+	 ssidrefs[1*STE_MAX_TYPES +j] = htons(1);
+     /* now set type j-1 for ssidref j */
+     for(j=0; j< STE_MAX_SSIDREFS; j++)
+	     if ((0 < j) &&( j <= STE_MAX_TYPES))
+		     ssidrefs[j*STE_MAX_TYPES + j - 1] = htons(1);
      ret += STE_MAX_TYPES*STE_MAX_SSIDREFS*sizeof(domaintype_t);
      return ret;
 }
diff -uprN xeno-unstable.bk.orig/tools/python/xen/lowlevel/xc/xc.c xeno-unstable.bk/tools/python/xen/lowlevel/xc/xc.c
--- xeno-unstable.bk.orig/tools/python/xen/lowlevel/xc/xc.c	2005-06-24 09:58:43.000000000 -0400
+++ xeno-unstable.bk/tools/python/xen/lowlevel/xc/xc.c	2005-06-25 14:18:25.000000000 -0400
@@ -78,7 +78,7 @@ static PyObject *pyxc_domain_create(PyOb
 
     u32          dom = 0;
     int          ret;
-    u32          ssidref = 0xFFFFFFFF;
+    u32          ssidref = 0x0;
 
     static char *kwd_list[] = { "dom", "ssidref", NULL };
 
diff -uprN xeno-unstable.bk.orig/tools/python/xen/xm/main.py xeno-unstable.bk/tools/python/xen/xm/main.py
--- xeno-unstable.bk.orig/tools/python/xen/xm/main.py	2005-06-22 12:07:48.000000000 -0400
+++ xeno-unstable.bk/tools/python/xen/xm/main.py	2005-06-25 14:18:25.000000000 -0400
@@ -399,9 +399,9 @@ class ProgList(Prog):
                 d['port'] = sxp.child_value(console, 'console_port')
             else:
                 d['port'] = ''
-            if ((int(sxp.child_value(info, 'ssidref', '-1'))) != -1):
-                d['ssidref1'] =  int(sxp.child_value(info, 'ssidref', '-1')) & 0xffff
-                d['ssidref2'] = (int(sxp.child_value(info, 'ssidref', '-1')) >> 16) & 0xffff
+            if ((int(sxp.child_value(info, 'ssidref', '0'))) != 0):
+                d['ssidref1'] =  int(sxp.child_value(info, 'ssidref', '0')) & 0xffff
+                d['ssidref2'] = (int(sxp.child_value(info, 'ssidref', '0')) >> 16) & 0xffff
                 print ("%(name)-16s %(dom)3d  %(mem)7d  %(cpu)3d  %(vcpus)5d   %(state)5s  %(cpu_time)7.1f     %(port)4s    s:%(ssidref2)02x/p:%(ssidref1)02x" % d)
             else:
                 print ("%(name)-16s %(dom)3d  %(mem)7d  %(cpu)3d  %(vcpus)5d   %(state)5s  %(cpu_time)7.1f     %(port)4s" % d)
diff -uprN xeno-unstable.bk.orig/xen/acm/acm_chinesewall_hooks.c xeno-unstable.bk/xen/acm/acm_chinesewall_hooks.c
--- xeno-unstable.bk.orig/xen/acm/acm_chinesewall_hooks.c	2005-06-21 11:32:08.000000000 -0400
+++ xeno-unstable.bk/xen/acm/acm_chinesewall_hooks.c	2005-06-25 14:18:25.000000000 -0400
@@ -50,7 +50,7 @@ int acm_init_chwall_policy(void)
 {
 	/* minimal startup policy; policy write-locked already */
 	chwall_bin_pol.max_types = 1;
-	chwall_bin_pol.max_ssidrefs = 1;
+	chwall_bin_pol.max_ssidrefs = 2;
 	chwall_bin_pol.max_conflictsets = 1;
 	chwall_bin_pol.ssidrefs = (domaintype_t *)xmalloc_array(domaintype_t, chwall_bin_pol.max_ssidrefs*chwall_bin_pol.max_types);
 	chwall_bin_pol.conflict_sets = (domaintype_t *)xmalloc_array(domaintype_t, chwall_bin_pol.max_conflictsets*chwall_bin_pol.max_types);
@@ -81,9 +81,10 @@ chwall_init_domain_ssid(void **chwall_ss
 	 * part of the global ssidref (same way we'll get the partial ssid pointer)
 	 */
 	chwall_ssidp->chwall_ssidref = GET_SSIDREF(ACM_CHINESE_WALL_POLICY, ssidref);
-	if (chwall_ssidp->chwall_ssidref >= chwall_bin_pol.max_ssidrefs) {
-		printkd("%s: ERROR chwall_ssidref(%x) > max(%x).\n",
-			__func__, chwall_ssidp->chwall_ssidref, chwall_bin_pol.max_ssidrefs-1);
+	if ((chwall_ssidp->chwall_ssidref >= chwall_bin_pol.max_ssidrefs) ||
+	    (chwall_ssidp->chwall_ssidref == ACM_DEFAULT_LOCAL_SSID)) {
+		printkd("%s: ERROR chwall_ssidref(%x) undefined (>max) or unset (0).\n",
+			__func__, chwall_ssidp->chwall_ssidref);
 		xfree(chwall_ssidp);
 		return ACM_INIT_SSID_ERROR;
 	}
diff -uprN xeno-unstable.bk.orig/xen/acm/acm_simple_type_enforcement_hooks.c xeno-unstable.bk/xen/acm/acm_simple_type_enforcement_hooks.c
--- xeno-unstable.bk.orig/xen/acm/acm_simple_type_enforcement_hooks.c	2005-06-23 10:20:03.000000000 -0400
+++ xeno-unstable.bk/xen/acm/acm_simple_type_enforcement_hooks.c	2005-06-25 14:18:25.000000000 -0400
@@ -73,14 +73,15 @@ int acm_init_ste_policy(void)
 {
 	/* minimal startup policy; policy write-locked already */
 	ste_bin_pol.max_types = 1;
-	ste_bin_pol.max_ssidrefs = 1;
-	ste_bin_pol.ssidrefs = (domaintype_t *)xmalloc_array(domaintype_t, 1);
-	
+	ste_bin_pol.max_ssidrefs = 2;
+	ste_bin_pol.ssidrefs = (domaintype_t *)xmalloc_array(domaintype_t, 2);
+	memset(ste_bin_pol.ssidrefs, 0, 2);
+
 	if (ste_bin_pol.ssidrefs == NULL)
 		return ACM_INIT_SSID_ERROR;
 
-	/* initialize state */
-	ste_bin_pol.ssidrefs[0] = 1;
+	/* initialize state so that dom0 can start up and communicate with itself */
+	ste_bin_pol.ssidrefs[1] = 1;
 
 	/* init stats */
 	atomic_set(&(ste_bin_pol.ec_eval_count), 0);
@@ -106,9 +107,10 @@ ste_init_domain_ssid(void **ste_ssid, ss
 
 	/* get policy-local ssid reference */
 	ste_ssidp->ste_ssidref = GET_SSIDREF(ACM_SIMPLE_TYPE_ENFORCEMENT_POLICY, ssidref);
-	if (ste_ssidp->ste_ssidref >= ste_bin_pol.max_ssidrefs) {
-		printkd("%s: ERROR ste_ssidref (%x) > max(%x).\n",
-			__func__, ste_ssidp->ste_ssidref, ste_bin_pol.max_ssidrefs-1);
+	if ((ste_ssidp->ste_ssidref >= ste_bin_pol.max_ssidrefs) ||
+	    (ste_ssidp->ste_ssidref == ACM_DEFAULT_LOCAL_SSID))	{
+		printkd("%s: ERROR ste_ssidref (%x) undefined or unset (0).\n",
+			__func__, ste_ssidp->ste_ssidref);
 		xfree(ste_ssidp);
 		return ACM_INIT_SSID_ERROR;
 	}
diff -uprN xeno-unstable.bk.orig/xen/include/acm/acm_hooks.h xeno-unstable.bk/xen/include/acm/acm_hooks.h
--- xeno-unstable.bk.orig/xen/include/acm/acm_hooks.h	2005-06-23 13:25:29.000000000 -0400
+++ xeno-unstable.bk/xen/include/acm/acm_hooks.h	2005-06-25 14:18:25.000000000 -0400
@@ -328,7 +328,7 @@ static inline int acm_pre_grant_setup(do
 }
 
 /* predefined ssidref for DOM0 used by xen when creating DOM0 */
-#define ACM_DOM0_SSIDREF        0
+#define ACM_DOM0_SSIDREF       0x00010001 
 
 static inline void acm_post_domain0_create(domid_t domid)
 {
diff -uprN xeno-unstable.bk.orig/xen/include/public/acm.h xeno-unstable.bk/xen/include/public/acm.h
--- xeno-unstable.bk.orig/xen/include/public/acm.h	2005-06-25 14:01:35.000000000 -0400
+++ xeno-unstable.bk/xen/include/public/acm.h	2005-06-25 14:18:25.000000000 -0400
@@ -41,8 +41,8 @@
 #endif
 
 /* default ssid reference value if not supplied */
-#define ACM_DEFAULT_SSID 	0xffffffff
-#define ACM_DEFAULT_LOCAL_SSID  0xffff
+#define ACM_DEFAULT_SSID 	0x0
+#define ACM_DEFAULT_LOCAL_SSID  0x0
 
 /* Internal ACM ERROR types */
 #define ACM_OK				 0
diff -uprN xeno-unstable.bk.orig/xen/include/public/policy_ops.h xeno-unstable.bk/xen/include/public/policy_ops.h
--- xeno-unstable.bk.orig/xen/include/public/policy_ops.h	2005-06-21 11:32:08.000000000 -0400
+++ xeno-unstable.bk/xen/include/public/policy_ops.h	2005-06-25 14:18:25.000000000 -0400
@@ -28,7 +28,7 @@
  * This makes sure that old versions of policy tools will stop working in a
  * well-defined way (rather than crashing the machine, for instance).
  */
-#define POLICY_INTERFACE_VERSION   0xAAAA0001
+#define POLICY_INTERFACE_VERSION   0xAAAA0002
 
 /************************************************************************/
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] 1 of 2: default ssid to 0

[Anthony Liguori]

Please make sure to increment the DOM0_INTERFACE_VERSION when this is 
committed.

Thanks,

Anthony Liguori

Stefan Berger wrote:

>Hello!
>
>Attached is the patch that changes the default ssid from 0xffffffff to 0 
>as discussed in previous emails. 
>
>Signed-off-by: Reiner Sailer <sailer@us.ibm.com>
>Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
>
>   Stefan
>
>
>  
>
>------------------------------------------------------------------------
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel
>  
>

Re: [PATCH] 1 of 2: default ssid to 0

[aq]

On 6/25/05, Stefan Berger <stefanb@us.ibm.com> wrote:
> Hello!
> 
> Attached is the patch that changes the default ssid from 0xffffffff to 0
> as discussed in previous emails.
> 
> Signed-off-by: Reiner Sailer <sailer@us.ibm.com>
> Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
> 
>    Stefan
> 



+     for(j=0; j<= CWALL_MAX_SSIDREFS; j++)
+         if ((0 < j) &&( j <= CWALL_MAX_TYPES))
+             ssidrefs[j*CWALL_MAX_TYPES + j - 1] = htons(1);
+

why dont you run the loop from j=1?


regards,
aq

Re: [PATCH] 1 of 2: default ssid to 0

[aq]

On 6/25/05, Stefan Berger <stefanb@us.ibm.com> wrote:
> Hello!
> 
> Attached is the patch that changes the default ssid from 0xffffffff to 0
> as discussed in previous emails.
> 
> Signed-off-by: Reiner Sailer <sailer@us.ibm.com>
> Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
> 
>    Stefan

looks like this patch (and also many other codes in the tree) mixed of
tabs and spaces for indentation :-(. especially this code will be in
mainline in the future.

is it time for us to have a official *coding style guide*, and all the
patches sent in must comply with the guide in order to be picked up
???


regards,
aq

Re: [PATCH] 1 of 2: default ssid to 0

[Mark Williamson]

On Sunday 26 June 2005 02:45, aq wrote:
> looks like this patch (and also many other codes in the tree) mixed of
> tabs and spaces for indentation :-(. especially this code will be in
> mainline in the future.
>
> is it time for us to have a official *coding style guide*, and all the
> patches sent in must comply with the guide in order to be picked up
> ???

It seems sensible for XenLinux code to follow the Linux CodingStyle document 
(although I'm not sure all the drivers do this yet).

Xen-related C code that isn't destined for the Linux kernel itself has a 
different coding style:
* we indent in blocks of 4 spaces and don't use tabs
* { and } are always on a new line, unlike K&R style
* we put a space between an if and the open parenthesis, and between the 
parentheses and the boolean: if ( murble )

There may be a few other things but I can't think of them right now ;-)

Fancy starting a Wiki page to collect coding style information?

Cheers,
Mark

Re: [PATCH] 1 of 2: default ssid to 0

[aq]

On 6/25/05, Mark Williamson <mark.williamson@cl.cam.ac.uk> wrote:
> On Sunday 26 June 2005 02:45, aq wrote:
> > looks like this patch (and also many other codes in the tree) mixed of
> > tabs and spaces for indentation :-(. especially this code will be in
> > mainline in the future.
> >
> > is it time for us to have a official *coding style guide*, and all the
> > patches sent in must comply with the guide in order to be picked up
> > ???
> 
> It seems sensible for XenLinux code to follow the Linux CodingStyle document
> (although I'm not sure all the drivers do this yet).
> 
> Xen-related C code that isn't destined for the Linux kernel itself has a
> different coding style:
> * we indent in blocks of 4 spaces and don't use tabs
> * { and } are always on a new line, unlike K&R style
> * we put a space between an if and the open parenthesis, and between the
> parentheses and the boolean: if ( murble )

i think it is very bad idea to have 2 coding styles for code in kernel
and out of kernel. because people work on all those code must always
remember to adjust the editor configuration, depend on the code in or
out of kernel, which is a nightmare :-(

how about this simple guide?
- C code complies Linux kernel coding style
- Python code can use (4) spaces for indentation (as in currently) 


regards,
aq

Re: [PATCH] 1 of 2: default ssid to 0

[Mark Williamson]

> i think it is very bad idea to have 2 coding styles for code in kernel
> and out of kernel. because people work on all those code must always
> remember to adjust the editor configuration, depend on the code in or
> out of kernel, which is a nightmare :-(

Yes, it's an extra thing to think about.  It also means that code imported 
from Linux needs adjusting to fit with the Xen coding convention.

OTOH, we don't just work with the Linux kernel any more, so there will be 
multiple coding conventions anyhow.  I just set up emacs so I can type "M-x 
xen-c-mode" or "M-x linux-c-mode".

> how about this simple guide?
> - C code complies Linux kernel coding style

I don't think this is likely to happen any time soon - it'd require patches to 
all of Xen and the tools :-( (and probably to some of the Linux code but 
that's arguably desirable)

> - Python code can use (4) spaces for indentation (as in currently)

Definitely a good plan!  Messing with Python's indentation rules is playing 
with fire...

Cheers,
Mark

Re: [PATCH] 1 of 2: default ssid to 0

[aq]

On 6/26/05, Mark Williamson <mark.williamson@cl.cam.ac.uk> wrote:
> > i think it is very bad idea to have 2 coding styles for code in kernel
> > and out of kernel. because people work on all those code must always
> > remember to adjust the editor configuration, depend on the code in or
> > out of kernel, which is a nightmare :-(
> 
> Yes, it's an extra thing to think about.  It also means that code imported
> from Linux needs adjusting to fit with the Xen coding convention.
> 
> OTOH, we don't just work with the Linux kernel any more, so there will be
> multiple coding conventions anyhow.  I just set up emacs so I can type "M-x
> xen-c-mode" or "M-x linux-c-mode".
> 
> > how about this simple guide?
> > - C code complies Linux kernel coding style
> 
> I don't think this is likely to happen any time soon - it'd require patches to
> all of Xen and the tools :-( (and probably to some of the Linux code but
> that's arguably desirable)
> 

yes, that is too bad. but at least if we have an official style, we
can enforce the rules *from now on*. the old code can be fix later (if
possible).

regards,
aq

Re: [PATCH] 1 of 2: default ssid to 0

[Stefan Berger]

xen-devel-bounces@lists.xensource.com wrote on 06/25/2005 09:49:18 PM:

> On Sunday 26 June 2005 02:45, aq wrote:
> > looks like this patch (and also many other codes in the tree) mixed of
> > tabs and spaces for indentation :-(. especially this code will be in
> > mainline in the future.
> >
> > is it time for us to have a official *coding style guide*, and all the
> > patches sent in must comply with the guide in order to be picked up
> > ???
> 
> It seems sensible for XenLinux code to follow the Linux CodingStyle 
document 
> (although I'm not sure all the drivers do this yet).
> 
> Xen-related C code that isn't destined for the Linux kernel itself has a 

> different coding style:
> * we indent in blocks of 4 spaces and don't use tabs

The drivers in the Linux sparse directory, are they falling under this 
category?

> * { and } are always on a new line, unlike K&R style
> * we put a space between an if and the open parenthesis, and between the 

> parentheses and the boolean: if ( murble )

If there was a script that we could run over the code, it would make life 
easier.

   Stefan

Re: [PATCH] 1 of 2: default ssid to 0

[Keir Fraser]

On 26 Jun 2005, at 17:10, Stefan Berger wrote:

> The drivers in the Linux sparse directory, are they falling under this
> category?

Yes, and I think at least some of them are now mostly Linux style. 
It'll be trivial to run them all through indent, plus a quick manual 
pass, before pushing upstream.

  -- Keir

Re: [PATCH] 1 of 2: default ssid to 0

[Chris Wright]

* Keir Fraser (Keir.Fraser@cl.cam.ac.uk) wrote:
> On 26 Jun 2005, at 17:10, Stefan Berger wrote:
> >The drivers in the Linux sparse directory, are they falling under this
> >category?
> 
> Yes, and I think at least some of them are now mostly Linux style. 
> It'll be trivial to run them all through indent, plus a quick manual 
> pass, before pushing upstream.

Agreed, I had done this a while back, but dropped it since it was a
large patch and seemed pointless at the time.

thanks,
-chris

Re: [PATCH] 1 of 2: default ssid to 0

[Josh Triplett]

On Sun, 2005-06-26 at 12:10 -0400, Stefan Berger wrote:
> xen-devel-bounces@lists.xensource.com wrote on 06/25/2005 09:49:18 PM:
> > * { and } are always on a new line, unlike K&R style
> > * we put a space between an if and the open parenthesis, and between the 
> > parentheses and the boolean: if ( murble )
> 
> If there was a script that we could run over the code, it would make life 
> easier.

I agree; I would suggest that in addition to the english description of
the coding style, there should be a list of the flags to pass to GNU
indent to make it generate the desired style.

- Josh Triplett