Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spawn bash shell in read_from_robot #20

Closed
JamesNewton opened this issue Jul 28, 2018 · 8 comments
Closed

Spawn bash shell in read_from_robot #20

JamesNewton opened this issue Jul 28, 2018 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@JamesNewton
Copy link
Collaborator

Extend the read_from_robot command to allow it to spawn a bash shell and inject a command (sent as the pathfile parameter) and return the stdout (and stderr?) text. This would allow greater interaction between the development environment and the operating system on Dexter. For example, DexRun.c could be updated and compiled, then run, by DDE.

@JamesNewton
Copy link
Collaborator Author

@JamesNewton
Copy link
Collaborator Author

Better, because it acts more like a file:
http://man7.org/linux/man-pages/man3/popen.3.html

@JamesNewton
Copy link
Collaborator Author

First take on adding the ability to spawn a command to the shell and return the result:

switch(token[0]) { //what's the first character of the path?
case '`': //shell cmd
	printf("shell %s\n", token);
	if(0==i){ //first request
		printf("- popen\n");
		wfp = popen(token, "r"); //to get both stdout and stderr, the command should end with "2>&1"
		if (errno) {
			sendBuffReTyped[5] = errno; //there was an error
			sendBuffReTyped[6] = 0; //no bytes returned
		}
	}
	if(wfp){
		sendBuffReTyped[6] = fread(sendBuff + sizeof(sendBuffReTyped[0])*7, 1, MAX_CONTENT_CHARS, wfp);
		//possible issue: If child process is working to produce more output, you might get less than
		//MAX_CONTENT_CHARS /before/ the child is done. Need to KEEP reading until you get EOF!
		//if(!sendBuffReTyped[6]) {//how do we know the command won't produce more later?
		if(feof(wfp)) { //stream will be set to eof when process is finished
			errno = pclose(wfp); //can this lock up? Shouldn't if the stream is eof
			sendBuffReTyped[5] = errno; //might be zero (no error) or the error returned by the command
		}
	}else { //printf("no wfp");
		sendBuffReTyped[5] = ECHILD; //we are done
		sendBuffReTyped[6] = 0; //no bytes returned
	}
	break;

I'm concerned about a case where the shell command is still working to produce output, but doesn't have any to send right now. e.g. we've read all the output and returned 0 characters. Or less than MAX_CONTENT_CHARS characters. So DDE thinks we are done. But the process is NOT finished. We only know the process is finished when we get an EOF on the stream. Then it's safe to call pclose. If we fail to call pclose, the process is still running, the handle is still open and the next time we try to execute a shell command, we crash. I could just call pclose when I'm told to do a new popen and wfp isn't null, but then that will lock DexRun up because pclose doesn't return until the child process is finished, AND... since nothing is reading the stream, it may not finish until the stream buffer is emptied, which creates a circular lock condition. Need to test. May have to change the way read_from_robot is called so that it keeps trying to get data until it gets back the ECHILD error indicating the process terminated.

@JamesNewton
Copy link
Collaborator Author

To avoid this problem, DDE would need to change so that it does NOT stop reading blocks when it gets less than MAX_CONTENT_CHARS returned. Instead, DDE would continue to read even if zero characters are returned, but would stop reading when an "END_OF_FILE" error is returned. E.g. Some error code that we will decide upon. This will be returned when the process stream has returned EOF, and pclose has been called, so we will need another way to return the error status from the bash command. We can update the standard read_from_robot as used for a file or #file to also return EOF when you try to read past the end of the file, so that DDE can do the same thing in all cases. E.g. We are changing the way that the 'r' oplet reports that it's finished to add an "I'm really, really finished" signal after the "there isn't thing to read" sort of "soft" ending we have now.

@JamesNewton
Copy link
Collaborator Author

Actually, we can just force one more round of reading back data AFTER the EOF which will return ECHILD (or perhaps we should make up another higher error number?) because the file handle will be null.

We should check for the file handle NOT being null on block zero and return an error so that the host knows a child process is still waiting to be cleared. Maybe EBUSY? Or a higher one we make up.

Here is the current code.

switch(token[0]) { //what's the first character of the path?
case '`': //shell cmd
	printf("shell %s\n", token);
	if(0==i){ //first request
		printf("- popen\n");
		if (wpp) { //we can't start a new one, 'case the old one isn't done.
			sendBuffReTyped[5] = EBUSY; //the system is busy
			sendBuffReTyped[6] = 0; //no bytes returned.
			break; //Host should request block 1 or higher with "`" and toss the data until EOF
		}
		wpp = popen(token, "r"); //to get both stdout and stderr, the command should end with "2>&1"
		if (errno) {
			sendBuffReTyped[5] = errno; //there was an error
			sendBuffReTyped[6] = 0; //no bytes returned
		}
	}
	if(wpp){
		sendBuffReTyped[6] = fread(sendBuff + sizeof(sendBuffReTyped[0])*7, 1, MAX_CONTENT_CHARS, wpp);
		//possible issue: If child process is working to produce more output, you might get less than
		//MAX_CONTENT_CHARS /before/ the child is done. Need to KEEP reading until you get EOF!
		if(feof(wpp)) { //stream will be set to eof when process is finished
			errno = pclose(wpp); //can this lock up? Shouldn't if the stream is eof
			sendBuffReTyped[5] = errno; //might be zero (no error) or the error returned by the command
		}
	}else { //printf("no wpp");
		sendBuffReTyped[5] = ECHILD; //we are done
		sendBuffReTyped[6] = 0; //no bytes returned
	}
	break;

@JamesNewton
Copy link
Collaborator Author

Error codes on this are going to be interesting. It turns out I was sending the back tick along with the command to popen. E.g. instead of sending "uname", I was sending "`uname" and it returned an error code of 512. The printf debug statement said "sh: 1: Syntax error: EOF in backquote substitution". I've NO idea where to find a list of error codes from Linux that goes up to 512.

Found the pclose does NOT zero out the file handle, so I had to add that myself.

Current code seems to work. I had to update the node web proxy index.html page to correctly decode 'r' return data and display it, and to console.log error codes so I could debug it as DDE is not currently able to send the commands as needed. I found a short Linux command which returns a short value: uname gives back Linux. So "r 0 `uname" returned r 6 Linux and "r 1 `uname" returned error 10 (ECHILD) as expected. And I was able to repeat that.

case '`': //shell cmd
	printf("shell %s\n", token);
	if(0==i){ //first request
		printf("- popen\n");
		if (wpp) { //we can't start a new one, 'case the old one isn't done.
			sendBuffReTyped[5] = EBUSY; //the system is busy
			sendBuffReTyped[6] = 0; //no bytes returned.
			break; //Host should request block 1 or higher with "`" and toss the data until EOF
		}
		wpp = popen(token+1, "r"); //to get both stdout and stderr, the command should end with "2>&1"
		if (errno) {
			sendBuffReTyped[5] = errno; //there was an error
			sendBuffReTyped[6] = 0; //no bytes returned
		}
	}
	if(wpp){
		sendBuffReTyped[6] = fread(sendBuff + sizeof(sendBuffReTyped[0])*7, 1, MAX_CONTENT_CHARS, wpp);
		//possible issue: If child process is working to produce more output, you might get less than
		//MAX_CONTENT_CHARS /before/ the child is done. Need to KEEP reading until you get EOF!
		if(feof(wpp)) { //printf("EOF\n");//stream will be set to eof when process is finished
			errno = pclose(wpp); //can this lock up? Shouldn't if the stream is eof
			sendBuffReTyped[5] = errno; //might be zero (no error) or the error returned by the command
			wpp = 0; //must zero out wpp so we know it's closed.
		}
	}else { //printf("no wpp");
		sendBuffReTyped[5] = ECHILD; //we are done
		sendBuffReTyped[6] = 0; //no bytes returned
	}
	break;

@JamesNewton
Copy link
Collaborator Author

This is completed and working as of 2018/08/16 on the StepAngles branch:
ce61cf6
with a bug fix (to allow multi word commands) in
104df3f

current code is:

case '`': //shell cmd
	//printf("shell %s\n", token);
	if(0==i){ //first request
		if (wpp) { //we can't start a new one, 'case the old one isn't done.
			sendBuffReTyped[5] = EBUSY; //the system is busy
			sendBuffReTyped[6] = 0; //no bytes returned.
			break; //Host should request block 1 or higher with "`" and toss the data until EOF
		}
		printf("popen %s \n",token+1);
		wpp = popen(token+1, "r"); //to get both stdout and stderr, the command should end with "2>&1"
		if (errno) {
			sendBuffReTyped[5] = errno; //there was an error
			sendBuffReTyped[6] = 0; //no bytes returned
		}
	}
	if(wpp){
		sendBuffReTyped[6] = fread(sendBuff + sizeof(sendBuffReTyped[0])*7, 1, MAX_CONTENT_CHARS, wpp);
		//possible issue: If child process is working to produce more output, you might get less than
		//MAX_CONTENT_CHARS /before/ the child is done. Need to KEEP reading until you get EOF!
		if(feof(wpp)) { //printf("EOF\n");//stream will be set to eof when process is finished
			errno = pclose(wpp); //can this lock up? Shouldn't if the stream is eof
			sendBuffReTyped[5] = errno; //might be zero (no error) or the error returned by the command
			wpp = 0; //must zero out wpp so we know it's closed.
		}
	}else { //printf("no wpp");
		sendBuffReTyped[5] = ECHILD; //we are done
		sendBuffReTyped[6] = 0; //no bytes returned
	}
	break;

But notice the change just before this:

		//token=strtok(NULL, delimiters); //this would get only one word, 
		token=strtok(NULL, ";"); // get the entire rest of the string. 
		//";" was already nulled, but it will stop at null

We used to strtok just the next word, but this change will cause it to grab the entire remainder of the line. This is necessary to support commands like ls /root/Documents where there are two or more "words" and it doesn't stop at the first space.

This, then, requires the following changes to the code for "#" keywords and actual files:

token = strtok(token, delimiters); //now get just the first word.

where we re-strtok the token pointer to get just the first word of the line. If we don't do this, commands like r 0 #Steps ; will fail because "#Steps " does not match "#Steps" and we can't be certain the user will NOT send that extra space at the end. Also for file names, Linux does not trim trailing spaces. It is entirely possible (though unlikely and evil) to have a file called "try-to-delete-me " which you can NOT delete without enclosing the name in quotes. e.g. rm try-to-delete-me will fail because it's missing the trailing space. Sadly, our current (and former) systems would never be able to read that file because they WILL stop at a space, even one inside quotes. Hopefully that is a non-issue. We could do a "r 0 `cat "try-to-delete-me"" and that should read back the file, assuming it's just standard text.

In any case, this shell ability seems to work and provides us with the ability to read directories, erase files, and cause all sort of mayhem in the onboard file system.

@JamesNewton
Copy link
Collaborator Author

Kamino cloned this issue to HaddingtonDynamics/OCADO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants