amigaos4: better popen() + pclose() implementation

popen(): handle better the case where the popened external
might exit before the child process manages to start.

pclose(): protect with a semaphore.

3 files changed, 81 insertions, 27 deletions

diff --git a/amigaos4/amigaio.c b/amigaos4/amigaio.c
index a5eb112400..40e9835d0f 100644
--- a/amigaos4/amigaio.c
+++ b/amigaos4/amigaio.c
@@ -18,6 +18,7 @@
 #include <exec/exectags.h>
 #include <proto/exec.h>
 #include <proto/dos.h>
+#include <proto/utility.h>
 #include <dos/dos.h>
 
 void amigaos_stdio_get(pTHX_ StdioStore *store)
@@ -66,10 +67,18 @@ PerlIO *Perl_my_popen(pTHX_ const char *cmd, const char *mode)
 	*/
 	//    FILE *f=amigaos_popen(cmd,mode);
 	//    fprintf(stderr,"popen returned %d\n",f);
-	return PerlIO_importFILE(amigaos_popen(cmd, mode), 0);
+	return PerlIO_importFILE(amigaos_popen(cmd, mode), mode);
 	//   return PerlIO_importFILE(f, 0);
 }
 
+I32 Perl_my_pclose(pTHX_ PerlIO *ptr)
+{
+	FILE * const f = PerlIO_findFILE(ptr);
+	const I32 result = amigaos_pclose(f);
+	PerlIO_releaseFILE(ptr,f);
+	return result;
+}
+
 #ifdef USE_ITHREADS
 
 /* An arbitrary number to start with, should work out what the real max should
@@ -747,7 +756,7 @@ int myexecve(bool isperlthread,
 		    (contains_whitespace(*cur) ? (2 + no_of_escapes(*cur)) : 0);
 	}
 	/* Check if it's a script file */
-
+	IExec->DebugPrintF("%s %ld %08lx %c %c\n",__FILE__,__LINE__,filename,filename[0],filename[1]);
 	fh = fopen(filename, "r");
 	if (fh)
 	{
@@ -839,7 +848,7 @@ int myexecve(bool isperlthread,
 
 				if (esc > 0)
 				{
-					char *buff = IExec->AllocVec(
+					char *buff = (char *)IExec->AllocVec(
 					                 strlen(*cur) + 4 + esc,
 					                 MEMF_ANY | MEMF_CLEAR);
 					char *p = *cur;
diff --git a/amigaos4/amigaos.c b/amigaos4/amigaos.c
index 44860c9101..67b4c065c4 100644
--- a/amigaos4/amigaos.c
+++ b/amigaos4/amigaos.c
@@ -241,10 +241,13 @@ char *convert_path_u2a(const char *filename)
 }
 
 static struct SignalSemaphore environ_sema;
+static struct SignalSemaphore popen_sema;
+
 
 void amigaos4_init_environ_sema()
 {
 	IExec->InitSemaphore(&environ_sema);
+	IExec->InitSemaphore(&popen_sema);
 }
 
 void amigaos4_obtain_environ()
@@ -310,6 +313,7 @@ struct command_data
 	struct Task *parent;
 };
 
+
 int myexecvp(bool isperlthread, const char *filename, char *argv[])
 {
 	//	adebug("%s %ld
@@ -518,16 +522,23 @@ void ___freeenviron()
 
 /* reimplementation of popen, clib2's doesn't do all we want */
 
+struct popen_data
+{
+	struct Task *parent;
+	STRPTR command;
+};
+
+static int popen_result = 0;
+
 int popen_child()
 {
 	struct Task *thisTask = IExec->FindTask(0);
-
-	char *command = (char *)thisTask->tc_UserData;
+	struct popen_data *pd = (struct popen_data *)thisTask->tc_UserData;
 	const char *argv[4];
 
 	argv[0] = "sh";
 	argv[1] = "-c";
-	argv[2] = command ? command : NULL;
+	argv[2] = pd->command ? pd->command : NULL;
 	argv[3] = NULL;
 
 	// adebug("%s %ld  %s\n",__FUNCTION__,__LINE__,command?command:"NULL");
@@ -535,11 +546,16 @@ int popen_child()
 	/* We need to give this to sh via execvp, execvp expects filename,
 	 * argv[]
 	 */
+	IExec->ObtainSemaphore(&popen_sema);
+
+	IExec->Signal(pd->parent,SIGBREAKF_CTRL_F);
 
-	myexecvp(FALSE, argv[0], (char **)argv);
-	if (command)
-		IExec->FreeVec(command);
+	popen_result = myexecvp(FALSE, argv[0], (char **)argv);
+	if (pd->command)
+		IExec->FreeVec(pd->command);
+	IExec->FreeVec(pd);
 
+	IExec->ReleaseSemaphore(&popen_sema);
 	IExec->Forbid();
 	return 0;
 }
@@ -551,11 +567,11 @@ FILE *amigaos_popen(const char *cmd, const char *mode)
 	char pipe_name[50];
 	char unix_pipe[50];
 	char ami_pipe[50];
-	char *cmd_copy;
 	BPTR input = 0;
 	BPTR output = 0;
 	struct Process *proc = NULL;
 	struct Task *thisTask = IExec->FindTask(0);
+	struct popen_data * pd = NULL;
 
 	/* First we need to check the mode
 	 * We can only have unidirectional pipes
@@ -585,15 +601,15 @@ FILE *amigaos_popen(const char *cmd, const char *mode)
 	sprintf(unix_pipe, "/PIPE/%s", pipe_name);
 	sprintf(ami_pipe, "PIPE:%s", pipe_name);
 
-	/* Now we open the AmigaOs filehandles that we will pass to our
-	 * subprocess
+	/* Now we open the AmigaOs Filehandles That we wil pass to our
+	 * Sub process
 	 */
 
 	if (mode[0] == 'r')
 	{
 		/* A read mode pipe: Output from pipe input from Output() or NIL:*/
 		/* First attempt to DUP Output() */
-		input = IDOS->DupFileHandle(IDOS->Output());
+		input = IDOS->DupFileHandle(IDOS->Input());
 		if(input == 0)
 		{
 			input = IDOS->Open("NIL:", MODE_READWRITE);
@@ -613,7 +629,7 @@ FILE *amigaos_popen(const char *cmd, const char *mode)
 		input = IDOS->Open(ami_pipe, MODE_OLDFILE);
 		if (input != 0)
 		{
-			output = IDOS->DupFileHandle(IDOS->Input());
+			output = IDOS->DupFileHandle(IDOS->Output());
 			if(output == 0)
 			{
 				output = IDOS->Open("NIL:", MODE_READWRITE);
@@ -643,29 +659,44 @@ FILE *amigaos_popen(const char *cmd, const char *mode)
 	 * no time in overwriting it! The subprocess will free the copy.
 	 */
 
-	if ((cmd_copy = mystrdup(cmd)))
+	if((pd = (struct popen_data*)IExec->AllocVecTags(sizeof(struct popen_data),AVT_Type,MEMF_SHARED,TAG_DONE)))
 	{
-		// adebug("%s %ld
-		// %s\n",__FUNCTION__,__LINE__,cmd_copy?cmd_copy:"NULL");
-		proc = IDOS->CreateNewProcTags(
-		           NP_Entry, popen_child, NP_Child, TRUE, NP_StackSize,
-		           ((struct Process *)thisTask)->pr_StackSize, NP_Input, input,
-		           NP_Output, output, NP_Error, IDOS->ErrorOutput(),
-		           NP_CloseError, FALSE, NP_Cli, TRUE, NP_Name,
-		           "Perl: popen process", NP_UserData, (int)cmd_copy,
-		           TAG_DONE);
+		pd->parent = thisTask;
+		if ((pd->command  = mystrdup(cmd)))
+		{
+			// adebug("%s %ld
+			// %s\n",__FUNCTION__,__LINE__,cmd_copy?cmd_copy:"NULL");
+			proc = IDOS->CreateNewProcTags(
+			           NP_Entry, popen_child, NP_Child, TRUE, NP_StackSize,
+			           ((struct Process *)thisTask)->pr_StackSize, NP_Input, input,
+			           NP_Output, output, NP_Error, IDOS->ErrorOutput(),
+			           NP_CloseError, FALSE, NP_Cli, TRUE, NP_Name,
+			           "Perl: popen process", NP_UserData, (int)pd,
+			           TAG_DONE);
+		}
+	}
+	if(proc)
+	{
+		/* wait for the child be setup right */
+		IExec->Wait(SIGBREAKF_CTRL_F);
 	}
 	if (!proc)
 	{
 		/* New Process Failed to start
 		 * Close and bail out
 		 */
+		if(pd)
+		{
+			if(pd->command)
+			{
+				IExec->FreeVec(pd->command);
+			}
+			IExec->FreeVec(pd);
+		}
 		if (input)
 			IDOS->Close(input);
 		if (output)
 			IDOS->Close(output);
-		if (cmd_copy)
-			IExec->FreeVec(cmd_copy);
 		if(result)
 		{
 			fclose(result);
@@ -680,7 +711,19 @@ FILE *amigaos_popen(const char *cmd, const char *mode)
 	return result;
 }
 
-/* Workaround for clib2 fstat */
+int amigaos_pclose(FILE *f)
+{
+	int result = -1;
+	/* close the file before obtaining the semaphore else we might end up
+	   hanging waiting for the child to read the last bit from the pipe */
+	fclose(f);
+	IExec->ObtainSemaphore(&popen_sema);
+	result = popen_result;
+	IExec->ReleaseSemaphore(&popen_sema);
+	return result;
+}
+
+/* Work arround for clib2 fstat */
 #ifndef S_IFCHR
 #define S_IFCHR 0x0020000
 #endif
diff --git a/amigaos4/amigaos.h b/amigaos4/amigaos.h
index 0faff8060c..4640bfac79 100644
--- a/amigaos4/amigaos.h
+++ b/amigaos4/amigaos.h
@@ -33,6 +33,8 @@ int myexecl(bool isperlthread, const char *path, ...);
 int pipe(int filedes[2]);
 
 FILE *amigaos_popen(const char *cmd, const char *mode);
+int   amigaos_pclose(FILE *f);
+
 void amigaos4_obtain_environ();
 void amigaos4_release_environ();