RE: [PATCH] fixdep: use fflush() and ferror() to ensure successful write to files

[David Laight <David.Laight@ACULAB.COM>]

From: Masahiro Yamada
> Sent: 21 February 2022 16:43
> To: linux-kbuild@vger.kernel.org
> 
> Checking the return value of (v)printf does not ensure the successful
> write to the .cmd file.
> 
> Call fflush() and ferror() to make sure that everything has been
> written to the file.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Reviewed-by: David Laight <dvid.laight@aculab.com>

I'll note that you've lost the perror("fixdep").
But I suspect that isn't very meaningful.
If the disk is full it'd probably get lost anyway.


> ---
> 
>  scripts/basic/fixdep.c | 44 ++++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 27 deletions(-)
> 
> diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
> index 44e887cff49b..fad6f29373a9 100644
> --- a/scripts/basic/fixdep.c
> +++ b/scripts/basic/fixdep.c
> @@ -105,25 +105,6 @@ static void usage(void)
>  	exit(1);
>  }
> 
> -/*
> - * In the intended usage of this program, the stdout is redirected to .*.cmd
> - * files. The return value of printf() must be checked to catch any error,
> - * e.g. "No space left on device".
> - */
> -static void xprintf(const char *format, ...)
> -{
> -	va_list ap;
> -	int ret;
> -
> -	va_start(ap, format);
> -	ret = vprintf(format, ap);
> -	if (ret < 0) {
> -		perror("fixdep");
> -		exit(1);
> -	}
> -	va_end(ap);
> -}
> -
>  struct item {
>  	struct item	*next;
>  	unsigned int	len;
> @@ -189,7 +170,7 @@ static void use_config(const char *m, int slen)
> 
>  	define_config(m, slen, hash);
>  	/* Print out a dependency path from a symbol name. */
> -	xprintf("    $(wildcard include/config/%.*s) \\\n", slen, m);
> +	printf("    $(wildcard include/config/%.*s) \\\n", slen, m);
>  }
> 
>  /* test if s ends in sub */
> @@ -318,13 +299,13 @@ static void parse_dep_file(char *m, const char *target)
>  				 */
>  				if (!saw_any_target) {
>  					saw_any_target = 1;
> -					xprintf("source_%s := %s\n\n",
> -						target, m);
> -					xprintf("deps_%s := \\\n", target);
> +					printf("source_%s := %s\n\n",
> +					       target, m);
> +					printf("deps_%s := \\\n", target);
>  				}
>  				is_first_dep = 0;
>  			} else {
> -				xprintf("  %s \\\n", m);
> +				printf("  %s \\\n", m);
>  			}
> 
>  			buf = read_file(m);
> @@ -347,8 +328,8 @@ static void parse_dep_file(char *m, const char *target)
>  		exit(1);
>  	}
> 
> -	xprintf("\n%s: $(deps_%s)\n\n", target, target);
> -	xprintf("$(deps_%s):\n", target);
> +	printf("\n%s: $(deps_%s)\n\n", target, target);
> +	printf("$(deps_%s):\n", target);
>  }
> 
>  int main(int argc, char *argv[])
> @@ -363,11 +344,20 @@ int main(int argc, char *argv[])
>  	target = argv[2];
>  	cmdline = argv[3];
> 
> -	xprintf("cmd_%s := %s\n\n", target, cmdline);
> +	printf("cmd_%s := %s\n\n", target, cmdline);
> 
>  	buf = read_file(depfile);
>  	parse_dep_file(buf, target);
>  	free(buf);
> 
> +	fflush(stdout);
> +
> +	/*
> +	 * In the intended usage, the stdout is redirected to .*.cmd files.
> +	 * Call ferror() to catch errors such as "No space left on device".
> +	 */
> +	if (ferror(stdout))
> +		exit(1);
> +
>  	return 0;
>  }
> --
> 2.32.0

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)