On Tue, Feb 22, 2022 at 7:33 AM David Laight <David.Laight@xxxxxxxxxx> wrote:
>
> From: Masahiro Yamada
> > Sent: 21 February 2022 16:43
> > To: linux-kbuild@xxxxxxxxxxxxxxx
> >
> > 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@xxxxxxxxxx>
>
> Reviewed-by: David Laight <dvid.laight@xxxxxxxxxx>
>
> 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.
perror() will go to stderr, i.e. tty here.
So, that is not the issue.
ferror() itself does not set errno here; "man ferror" says,
"These functions should not fail and do not set the external
variable errno"
So, I dropped perror() because I am not sure if any related error
message is printed here.
Perhaps, errno was set by some of preceding printf() calls,
but I am not quite sure if it is carried all the way to the end
of this program.
>
> > ---
> >
> > 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)
>
--
Best Regards
Masahiro Yamada
