[Debootloaders-devel] r92 - trunk/rsrce

[Jeremie Koenig]

Author: jkoenig-guest
Date: 2006-07-12 07:09:44 +0000 (Wed, 12 Jul 2006)
New Revision: 92

Modified:
   trunk/rsrce/command.c
Log:
- made "edit" more careful with handling errors and not leaving cruft behind in
  /tmp.
- return values from command functions should be positive: we'd like to be able
  to exit() with them at some point;


Modified: trunk/rsrce/command.c
===================================================================
--- trunk/rsrce/command.c	2006-07-12 06:33:52 UTC (rev 91)
+++ trunk/rsrce/command.c	2006-07-12 07:09:44 UTC (rev 92)
@@ -74,19 +74,20 @@
 	
 	if(!argv[1]) {
 		fprintf(stderr, "Read which file ?\n");
-		return -1;
+		return 1;
 	}
 
 	f = fopen(argv[1], "r");
 	if(!f) {
 		perror(argv[1]);
-		return -1;
+		return 1;
 	}
 
 	rf = res_read(f);
+	fclose(f);
 	if(!rf) {
 		fprintf(stderr, "Error while reading %s\n", argv[1]);
-		return -1;
+		return 1;
 	}
 
 	res_delfork(cmd_rf);
@@ -97,21 +98,24 @@
 int cmd_write(char **argv)
 {
 	FILE *f;
-	
+	int ret;
+
 	if(!argv[1]) {
 		fprintf(stderr, "Write to which file ?\n");
-		return -1;
+		return 1;
 	}
 
 	f = fopen(argv[1], "w");
 	if(!f) {
 		perror(argv[1]);
-		return -1;
+		return 1;
 	}
 
-	if(res_write(cmd_rf, f)) {
+	ret = res_write(cmd_rf, f);
+	fclose(f);
+	if(ret < 0) {
 		fprintf(stderr, "Write error\n");
-		return -1;
+		return 1;
 	}
 
 	return 0;
@@ -121,16 +125,16 @@
 {
 	if(!argv[1]) {
 		fprintf(stderr, "%s <resource>\n", argv[0]);
-		return -1;
+		return 1;
 	}
 
 	if(cmd_parseresource(argv[1]))
-		return -1;
+		return 1;
 
 	cmd_res = res_lookup(cmd_rf, cmd_res_type, cmd_res_id);
 	if(cmd_res) {
 		fprintf(stderr, "Resource already exists.\n");
-		return -1;
+		return 1;
 	}
 
 	cmd_res = res_new(cmd_rf, cmd_res_type, cmd_res_id);
@@ -141,11 +145,11 @@
 {
 	if(!argv[1]) {
 		fprintf(stderr, "%s <resource>\n", argv[0]);
-		return -1;
+		return 1;
 	}
 
 	if(cmd_selectresource(argv[1]))
-		return -1;
+		return 1;
 
 	res_delete(cmd_res);
 	return 0;
@@ -155,11 +159,11 @@
 {
 	if(!argv[2]) {
 		fprintf(stderr, "%s <resource> <name>\n", argv[0]);
-		return -1;
+		return 1;
 	}
 
 	if(cmd_selectresource(argv[1]))
-		return -1;
+		return 1;
 
 	res_rename(cmd_res, argv[2], strlen(argv[2]));
 	return 0;
@@ -169,11 +173,11 @@
 {
 	if(!argv[2]) {
 		fprintf(stderr, "%s <resource> <attributes>\n", argv[0]);
-		return -1;
+		return 1;
 	}
 
 	if(cmd_selectresource(argv[1]))
-		return -1;
+		return 1;
 
 	res_chattr(cmd_res, argv[2]);
 	return 0;
@@ -225,7 +229,7 @@
 	
 	if(!argv[2]) {
 		fprintf(stderr, "%s <resource> <file> [<type>]\n", argv[0]);
-		return -1;
+		return 1;
 	}
 	if(!argv[3]) {
 		c = strrchr(argv[2], '.');
@@ -233,17 +237,19 @@
 	}
 
 	if(cmd_selectresource(argv[1]))
-		return -1;
+		return 1;
 	
 	tr = tr_lookup(cmd_res, argv[3]);
-	if(!tr)
-		return -1;
-	
+	if(!tr) {
+		fprintf(stderr, "I don't know how to translate this.\n");
+		return 1;
+	}
+
 	f = (strcmp(argv[2], "-") == 0) ? (what ? stdout : stdin)
 					: fopen(argv[2], what ? "w" : "r");
 	if(!f) {
 		perror(argv[2]);
-		return -1;
+		return 1;
 	}
 
 	r = (what ? tr_export : tr_import)(tr, cmd_res, f);
@@ -256,30 +262,37 @@
 /* Export the resource data to a temporary file and return its name */
 static char *cmd_edit_export(struct translator *tr)
 {
-	char filename[] = "/tmp/rsrce.XXXXXX", *nfname;
+	char filename[] = "/tmp/rsrce.XXXXXX", *nfname, *ret = NULL;
 	const char *ext;
-	int fd;
-	FILE *f;
+	int fd, r;
+	FILE *f = NULL;
 	
 	fd  = mkstemp(filename);
+	if(fd < 0) {
+		perror(filename);
+		return NULL;
+	}
+
 	ext = tr_ext(tr);
 	nfname = malloc(strlen(filename) + strlen(ext) + 2);
 	sprintf(nfname, "%s.%s", filename, ext);
-	if(rename(filename, nfname) != 0 || (f = fdopen(fd, "w")) == NULL) {
+
+	if((r = rename(filename, nfname)) < 0) 
 		perror(nfname);
-		close(fd);
+	else if((f = fdopen(fd, "w")) == NULL)
+		perror("fdopen");
+	else if(tr_export(tr, cmd_res, f) < 0)
+		fprintf(stderr,"Couldn't translate resource data\n");
+	else
+		ret = nfname;
+
+	if(!ret) {
+		unlink(r < 0 ? filename : nfname);
 		free(nfname);
-		return NULL;
 	}
-	if(tr_export(tr, cmd_res, f) != 0) {
-		fprintf(stderr,"Couldn't export resource data to %s\n",nfname);
-		fclose(f);
-		free(nfname);
-		return NULL;
-	}
-	fclose(f);
+	if(f) fclose(f); else close(fd);
 
-	return nfname;
+	return ret;
 }
 
 /* Invoke the external editor */
@@ -288,6 +301,8 @@
 	char *cmd;
 	int ret;
 
+	/* FIXME: We should fork and exec rather than system in order to avoid
+	 * shell interference with funny filenames. */
 	cmd = malloc(strlen(CFG_EDITOR) + strlen(fname) + 2);
 	sprintf(cmd, "%s %s", CFG_EDITOR, fname);
 	ret = system(cmd);
@@ -310,7 +325,6 @@
 
 	ret = tr_import(tr, cmd_res, f);
 	fclose(f);
-	unlink(filename);
 
 	return ret;
 }
@@ -325,10 +339,16 @@
 		return 1;
 
 	tr = tr_lookup(cmd_res, argv[2]);
+	if(!tr) {
+		fprintf(stderr, "I don't know how to translate this.\n");
+		return 1;
+	}
+
 	fname = cmd_edit_export(tr);
 	if(!fname)
 		return 1;
 	ret = (cmd_edit_edit(fname) == 0 && cmd_edit_import(tr, fname) == 0);
+	unlink(fname);
 	free(fname);
 
 	return ret ? 0 : 1;
@@ -406,7 +426,7 @@
 
 	if(argv[MAXARGC]) {
 		fprintf(stderr, "Too many arguments\n");
-		return -1;
+		return 1;
 	}
 
 	for(i=0 ; cmd_table[i].cmd ; i++) {
@@ -415,7 +435,7 @@
 	}
 
 	fprintf(stderr, "%s: no such command\n", argv[0]);
-	return -1;
+	return 1;
 }
 
 void cmd_init(FILE *f)