diff mbox

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

Message ID 1373587565-15149-1-git-send-email-colin.king@canonical.com
State Accepted
Headers show

Commit Message

Colin Ian King July 12, 2013, 12:06 a.m. UTC
From: Colin Ian King <colin.king@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@canonical.com>
---
 src/acpica/source/compiler/fwts_iasl_interface.c | 64 +++++++++++++++---------
 1 file changed, 40 insertions(+), 24 deletions(-)

Comments

Alex Hung July 12, 2013, 2:41 a.m. UTC | #1
On 07/12/2013 08:06 AM, Colin King wrote:
> From: Colin Ian King <colin.king@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@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;
>   }
>
Acked-by: Alex Hung <alex.hung@canonical.com>
Colin Ian King July 12, 2013, 9:05 a.m. UTC | #2
On 12/07/13 01:06, Colin King wrote:
> From: Colin Ian King <colin.king@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@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
Colin Ian King July 12, 2013, 10:13 a.m. UTC | #3
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@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@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
Keng-Yu Lin July 26, 2013, 3:57 a.m. UTC | #4
On Fri, Jul 12, 2013 at 8:06 AM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@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@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;
>  }
> --
> 1.8.1.2
>

Acked-by: Keng-Yu Lin <kengyu@canonical.com>
diff mbox

Patch

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;
 }