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

[Alex Hung]

On 07/12/2013 08:06 AM, 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;
>   }
>
Acked-by: Alex Hung <alex.hung at canonical.com>

-- 
Cheers,
Alex Hung