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

[Masahiro Yamada]

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