On Tue, Mar 21, 2006 at 11:46:10 +1100, Brian May wrote:
> I noticed your patch to this problem, so I decided to review it.
Thanks.
> It looks good to me. Other solutions are also possible, but your
> solution looks better.
>
> However, I notice the cleanup() routine still calls pclose(...). This has
> me puzzled because no errors are generated, so maybe the cleanup code
> isn't being called for some reason (it looks like gs==null???).
>
> I think pclose (which is documented only to work with popen) needs to be
> replaced with waitpid(...). This function should also perform the
> cleanup you were concerned about (not really a major issue with pstotext
> because it exits so quickly anyway - but it seems like a good idea to
> get the exit status). For this to work, you will need to save the PID
> returned by fork so cleanup can access it.
I'm attaching an updated version of my patch. Note that I'm not explicitly
saving the child PID yet, but it might just do the trick.
> As for VMS, this is something I have no idea what-so-ever about. I would
> assume it should work, but I have never been near a VMS system so I have
> no idea.
Frankly, I don't think anyone cares about the VMS port. I'm just trying to
make minimal changes at this point to make it easier to integrate this
upstream.
Ray
--
FUD for dummies by example in 21 days Lesson 3: use braindead analogies.
"Open source raises various security issues. How many banks will tell you
where their cameras are and where their vaults are?"
A Microsoft droid in http://www.zdnet.co.uk/news/1999/47/ns-11895.html
diff -ru ../../pstotext-1.9/main.c ./main.c
--- ../../pstotext-1.9/main.c 2005-07-24 18:56:11.000000000 +0200
+++ ./main.c 2006-03-21 21:55:55.000000000 +0100
@@ -126,12 +126,14 @@
static int cleanup(void) {
int gsstatus, status = 0;
pstotextExit(instance);
- if (gs!=NULL) {
#ifdef VMS
+ if (gs!=NULL) {
gsstatus = fclose(gs);
+ }
#else
- gsstatus = pclose(gs);
+ waitpid(-1, &gsstatus, 0);
#endif
+ if (gsstatus) {
if (WIFEXITED(gsstatus)) {
if (WEXITSTATUS(gsstatus)!=0) status = 3;
else if (WIFSIGNALED(gsstatus)) status = 4;
@@ -166,7 +168,13 @@
static int do_it(char *path) {
/* If "path" is NULL, then "stdin" should be processed. */
- char *gs_cmdline;
+ char *gs_argv[32];
+ int gs_argc=0;
+#ifdef DEBUG
+ int i;
+#endif
+ int fd[2];
+ pid_t p;
char *input;
int status;
char norotate[] = "";
@@ -216,32 +224,29 @@
cleanup();
exit(1);
}
- strcpy(input, "-- '"); strcat(input, path); strcat(input, "'");
+ strcpy(input, path);
}
- gs_cmdline = (char*)malloc(strlen(gs_cmd)+strlen(rotate_path)+
- strlen(ocr_path) + strlen(input) + 128);
-
- if (gs_cmdline == NULL) {
- fprintf(stderr, "No memory available\n");
- cleanup();
- exit(1);
- }
-
- sprintf(
- gs_cmdline,
-#ifdef VMS
- "%s -r72 \"-dNODISPLAY\" \"-dFIXEDMEDIA\" \"-dDELAYBIND\"
\"-dWRITESYSTEMDICT\" %s \"-dNOPAUSE\" \"-dSAFER\" %s %s %s",
-#else
- "%s -r72 -dNODISPLAY -dFIXEDMEDIA -dDELAYBIND -dWRITESYSTEMDICT %s
-dNOPAUSE -dSAFER %s %s %s",
-#endif
- gs_cmd,
- (debug ? "" : "-q"),
- rotate_path,
- ocr_path,
- input
- );
- if (debug) fprintf(stderr, "%s\n", gs_cmdline);
+ gs_argv[gs_argc++] = "gs";
+ gs_argv[gs_argc++] = "-r72";
+ gs_argv[gs_argc++] = "-dNODISPLAY";
+ gs_argv[gs_argc++] = "-dFIXEDMEDIA";
+ gs_argv[gs_argc++] = "-dDELAYBIND";
+ gs_argv[gs_argc++] = "-dWRITESYSTEMDICT";
+ if (!debug) {
+ gs_argv[gs_argc++] = "-q";
+ }
+ gs_argv[gs_argc++] = "-dNOPAUSE";
+ gs_argv[gs_argc++] = "-dSAFER";
+ if (rotate_path && strcmp(rotate_path, "")) {
+ gs_argv[gs_argc++] = rotate_path;
+ }
+ if (ocr_path && strcmp(ocr_path, "")) {
+ gs_argv[gs_argc++] = ocr_path;
+ }
+ gs_argv[gs_argc++] = "--";
+ gs_argv[gs_argc++] = input;
+ gs_argv[gs_argc++] = NULL;
#ifdef VMS
cmdfile = tempnam("SYS$SCRATCH:","PS2TGS");
gsoutfile = tempnam("SYS$SCRATCH:","GSRES");
@@ -259,8 +264,25 @@
exit(1);
}
#else
- gs = popen(gs_cmdline, "r");
- if (gs==0) {perror(cmd); exit(1);}
+ if (pipe(fd)) {
+ perror("pipe failed: "); exit(1);
+ };
+ p = fork();
+ if (p == -1) {
+ perror("fork failed: "); exit(1);
+ }
+ if (p == 0) { /* child */
+ close(fd[0]);
+ dup2(fd[1], 1); /* Redirect stdout into pipe to parent */
+ execvp("/usr/bin/gs", gs_argv);
+ perror("execvp: "); status=cleanup(); exit(1);
+ } else { /* parent */
+ close(fd[1]);
+ gs = fdopen(fd[0], "r");
+ if (gs == NULL) {
+ perror("fdopen: "); status=cleanup(); exit(1);
+ }
+ }
#endif
status = pstotextInit(&instance);
if (status!=0) {
