[PATCH] fwts_iasl_interface.c: ensure we read in all of stdout (LP: #1200426)

[Colin Ian King]

On 12/07/13 10:05, Colin Ian King wrote:
> On 12/07/13 01:06, Colin King wrote:
>> From: Colin Ian King <colin.king at canonical.com>
>>
>> Redirecting stdout to a file when running the syntaxcheck test results
>> in no failures.  We need to ensure we fflush on stdout to flush the
>> IASL output on stdout down the pipe to the reading parent process.
>>
>> This patch also ensures we check for NULL on realloc.  Also fixes
>> an incorrect stdout fd check on pipefds[0] and removes an unnecessary
>> close on pipefds[1] on the child process.
>>
>> Took the opportunity to do some white space cleanup and a general tidy
>> up of the code too.  Must of had a bad day when I originally wrote
>> this code.
>>
>> Signed-off-by: Colin Ian King <colin.king at canonical.com>
>> ---
>>  src/acpica/source/compiler/fwts_iasl_interface.c | 64 +++++++++++++++---------
>>  1 file changed, 40 insertions(+), 24 deletions(-)
>>
>> diff --git a/src/acpica/source/compiler/fwts_iasl_interface.c b/src/acpica/source/compiler/fwts_iasl_interface.c
>> index 84670e4..206e0c8 100644
>> --- a/src/acpica/source/compiler/fwts_iasl_interface.c
>> +++ b/src/acpica/source/compiler/fwts_iasl_interface.c
>> @@ -33,7 +33,7 @@ static void init_asl_core(void)
>>  
>>  	AcpiDbgLevel = 0;
>>  
>> -	for (i=0; i<ASL_NUM_FILES; i++) {
>> +	for (i = 0; i < ASL_NUM_FILES; i++) {
>>  		Gbl_Files[i].Handle = NULL;
>>  		Gbl_Files[i].Filename = NULL;
>>  	}
>> @@ -64,8 +64,8 @@ int fwts_iasl_disassemble_aml(const char *aml, const char *outputfile)
>>  
>>  		/* Setup ACPICA disassembler globals */
>>  		Gbl_DisasmFlag = TRUE;
>> -        	Gbl_DoCompile = FALSE;
>> -        	Gbl_OutputFilenamePrefix = (char*)outputfile;
>> +		Gbl_DoCompile = FALSE;
>> +		Gbl_OutputFilenamePrefix = (char*)outputfile;
>>  		Gbl_UseDefaultAmlFilename = FALSE;
>>  
>>  		/* Throw away noisy errors */
>> @@ -76,7 +76,7 @@ int fwts_iasl_disassemble_aml(const char *aml, const char *outputfile)
>>  		break;
>>  	default:
>>  		/* Parent */
>> -		waitpid(pid, &status, WUNTRACED | WCONTINUED);
>> +		(void)waitpid(pid, &status, WUNTRACED | WCONTINUED);
>>  	}
>>  
>>  	return 0;
>> @@ -84,13 +84,14 @@ int fwts_iasl_disassemble_aml(const char *aml, const char *outputfile)
>>  
>>  int fwts_iasl_assemble_aml(const char *source, char **output)
>>  {
>> -	int pipefds[2];
>> +	int 	pipefds[2];
>> +	int	status;
>> +	int 	ret = 0;
>> +	size_t	len = 0;
>> +	ssize_t	n;
>>  	pid_t	pid;
>>  	char    *data = NULL;
>>  	char	buffer[8192];
>> -	int	n;
>> -	int 	len = 0;
>> -	int	status;
>>  
>>  	if (pipe(pipefds) < 0)
>>  		return -1;
>> @@ -98,46 +99,61 @@ int fwts_iasl_assemble_aml(const char *source, char **output)
>>  	pid = fork();
>>  	switch (pid) {
>>  	case -1:
>> -		close(pipefds[0]);
>> -		close(pipefds[1]);
>> +		(void)close(pipefds[0]);
>> +		(void)close(pipefds[1]);
>>  		return -1;
>>  	case 0:
>>  		/* Child */
>>  		init_asl_core();
>>  
>> -		if (pipefds[0] != STDOUT_FILENO) {
>> -			dup2(pipefds[1], STDOUT_FILENO);
>> -			close(pipefds[1]);
>> +		/* stdout to be redirected down the writer end of pipe */
>> +		if (pipefds[1] != STDOUT_FILENO) {
>> +			if (dup2(pipefds[1], STDOUT_FILENO) < 0) {
>> +				_exit(EXIT_FAILURE);
>> +			}
>>  		}
>> -		close(pipefds[0]);
>> +		/* Close reader end */
>> +		(void)close(pipefds[0]);
>>  
>>  		/* Setup ACPICA compiler globals */
>>  		Gbl_DisasmFlag = FALSE;
>> -        	Gbl_DoCompile = TRUE;
>> +		Gbl_DoCompile = TRUE;
>>  		Gbl_PreprocessFlag = TRUE;
>> -        	Gbl_UseDefaultAmlFilename = FALSE;
>> -        	Gbl_OutputFilenamePrefix = (char*)source;
>> +		Gbl_UseDefaultAmlFilename = FALSE;
>> +		Gbl_OutputFilenamePrefix = (char*)source;
>>  
>> -		status = AslDoOnePathname((char*)source, AslDoOneFile);
>> +		(void)AslDoOnePathname((char*)source, AslDoOneFile);
>>  
>> -		close(pipefds[1]);
>> +		/*
>> +		 * We need to flush buffered I/O on IASL stdout
>> +		 * before we exit
>> +		 */
>> +		fflush(stdout);
>>  
>>  		_exit(0);
>>  		break;
>>  	default:
>>  		/* Parent */
>> -		close(pipefds[1]);
>>  
>> +		/* Close writer end */
>> +		(void)close(pipefds[1]);
>> +
>> +		/* Gather output from IASL */
>>  		while ((n = read(pipefds[0], buffer, sizeof(buffer))) > 0) {
>>  			data = realloc(data, len + n + 1);
>> +			if (data == NULL) {
>> +				ret = -1;
>> +				break;
>> +			}
>>  			memcpy(data + len, buffer, n);
>>  			len += n;
>> -			data[len] = 0;
>> +			data[len] = '\0';
>>  		}
>> -		waitpid(pid, &status, WUNTRACED | WCONTINUED);
>> -		close(pipefds[0]);
>> +		(void)waitpid(pid, &status, WUNTRACED | WCONTINUED);
>> +		(void)close(pipefds[0]);
>> +		break;
>>  	}
>>  
>>  	*output = data;
>> -	return 0;
>> +	return ret;
>>  }
>>
> 
> NACK, found a regression on some deeper testing. Will sent an update
> later.  Apologies
> 

Sorry, ignore my own NACK, I found the bug was somewhere else. This code
is working as expected after all. *sigh*

Colin