Re: VKERNEL Pidfile patch

[Chris Turner <c.turner@xxxxxxxxxxxxxxxxxxx>]

Matthew Dillon wrote:
>     if ((fp = fopen(pid_file, "w")) != NULL) {
> 	    do stuff
> 	    do stuff
>     } else {
> 	    perror(...)
>     }
> 
> 	I still do this.  Some people really hate it and for a complex
> 	operation it probably does end up being less readable.  If it 
> 	doesn't break the line, though, I consider it ok to still use this
> 	construct at least for simple code paths.

Nearly did this on the first round,  but thought it wasn't as legible,
and wasn't sure what the consensus here was..

As on occasional perl programmer, I like my statements short and cryptic
:)

Okay.. whew. fine-tooth-comb here..

but it does look much cleaner, I will admit.

Thanks for the reviews, esp for such a simple patch.

- Chris

Index: share/man/man7/vkernel.7
===================================================================
RCS file: /var/local/apps/dcvs/src/share/man/man7/vkernel.7,v
retrieving revision 1.18
diff -u -p -r1.18 vkernel.7
--- share/man/man7/vkernel.7	14 Jun 2007 21:11:29 -0000	1.18
+++ share/man/man7/vkernel.7	17 Jun 2007 04:05:58 -0000
@@ -51,6 +51,7 @@ .Op Fl e Ar name Ns = Ns Li value : Ns A
 .Op Fl i Ar file
 .Op Fl I Ar interface Ns Op Ar :address1 Ns Oo Ar :address2 Oc Ns Oo Ar /netmask Oc
 .Op Fl m Ar size
+.Op Fl p Ar file
 .Op Fl r Ar file
 .Sh DESCRIPTION
 The
@@ -149,6 +150,9 @@ .Cm K , M ,
 and
 .Cm G
 are allowed.
+.It Fl p Ar file
+Specify a file in which to store the process ID. 
+A warning is issued if this file cannot be opened for writing.
 .It Fl r Ar file
 Specify a R/W disk image
 .Ar file
Index: sys/platform/vkernel/platform/init.c
===================================================================
RCS file: /var/local/apps/dcvs/src/sys/platform/vkernel/platform/init.c,v
retrieving revision 1.39
diff -u -p -r1.39 init.c
--- sys/platform/vkernel/platform/init.c	28 May 2007 05:26:29 -0000	1.39
+++ sys/platform/vkernel/platform/init.c	19 Jun 2007 00:39:57 -0000
@@ -80,6 +80,7 @@ 
 int DiskNum;
 struct vknetif_info NetifInfo[VKNETIF_MAX];
 int NetifNum;
+char *pid_file;
 vm_offset_t KvaStart;
 vm_offset_t KvaEnd;
 vm_offset_t KvaSize;
@@ -107,6 +108,8 @@ 
 static void init_vkernel(void);
 static void init_disk(char *diskExp[], int diskFileNum, enum vkdisk_type type); 
 static void init_netif(char *netifExp[], int netifFileNum);
+static void writepid( void );
+static void cleanpid( void );
 static void usage(const char *ctl);
 
 static int save_ac;
@@ -129,7 +132,7 @@ 	int cdFileNum = 0;
 	int c;
 	int i;
 	int n;
-
+	
 	save_ac = ac;
 	save_av = av;
 
@@ -138,7 +141,7 @@ 	 * Process options
 	 */
 	kernel_mem_readonly = 1;
 
-	while ((c = getopt(ac, av, "c:svm:r:e:i:I:U")) != -1) {
+	while ((c = getopt(ac, av, "c:svm:r:e:i:p:I:U")) != -1) {
 		switch(c) {
 		case 'e':
 			/*
@@ -200,12 +203,16 @@ 					break;
 				}
 			}
 			break;
+		case 'p':
+			pid_file = optarg;	
+			break;
 		case 'U':
 			kernel_mem_readonly = 0;
 			break;
 		}
 	}
 
+	writepid();
 	cpu_disable_intr();
 	init_sys_memory(memImageFile);
 	init_kern_memory();
@@ -1039,6 +1046,37 @@ }
 
 static
 void
+writepid( void )
+{
+	pid_t self;
+	FILE *fp;
+
+	if (pid_file != NULL) {
+		self = getpid();
+		fp = fopen(pid_file, "w");
+
+		if (fp != NULL) {
+			fprintf(fp, "%ld\n", (long)self);
+			fclose(fp);
+		}
+		else {
+			perror("Warning: couldn't open pidfile");
+		}
+	}
+}
+
+static
+void
+cleanpid( void ) 
+{
+	if (pid_file != NULL) {
+		if ( unlink(pid_file) != 0 )
+			perror("Warning: couldn't remove pidfile");
+	}
+}
+
+static
+void
 usage(const char *ctl)
 {
 	
@@ -1049,6 +1087,7 @@ 
 {
 	kprintf("cpu reset, rebooting vkernel\n");
 	closefrom(3);
+	cleanpid();
 	execv(save_av[0], save_av);
 }
 
@@ -1056,5 +1095,6 @@ 
 cpu_halt(void)
 {
 	kprintf("cpu halt, exiting vkernel\n");
+	cleanpid();
 	exit(0);
 }