* [PATCH] fixdep: use fflush() and ferror() to ensure successful write to files
@ 2022-02-21 16:43 Masahiro Yamada
2022-02-21 22:33 ` David Laight
0 siblings, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2022-02-21 16:43 UTC (permalink / raw)
To: linux-kbuild
Cc: David Laight, linux-kernel, Masahiro Yamada, Michal Marek,
Nick Desaulniers
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>
---
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
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH] fixdep: use fflush() and ferror() to ensure successful write to files
2022-02-21 16:43 [PATCH] fixdep: use fflush() and ferror() to ensure successful write to files Masahiro Yamada
@ 2022-02-21 22:33 ` David Laight
2022-02-22 3:44 ` Masahiro Yamada
0 siblings, 1 reply; 4+ messages in thread
From: David Laight @ 2022-02-21 22:33 UTC (permalink / raw)
To: 'Masahiro Yamada', linux-kbuild
Cc: linux-kernel, Michal Marek, Nick Desaulniers
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)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] fixdep: use fflush() and ferror() to ensure successful write to files
2022-02-21 22:33 ` David Laight
@ 2022-02-22 3:44 ` Masahiro Yamada
2022-02-22 9:09 ` David Laight
0 siblings, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2022-02-22 3:44 UTC (permalink / raw)
To: David Laight; +Cc: linux-kbuild, linux-kernel, Michal Marek, Nick Desaulniers
On Tue, Feb 22, 2022 at 7:33 AM David Laight <David.Laight@aculab.com> wrote:
>
> 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.
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
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH] fixdep: use fflush() and ferror() to ensure successful write to files
2022-02-22 3:44 ` Masahiro Yamada
@ 2022-02-22 9:09 ` David Laight
0 siblings, 0 replies; 4+ messages in thread
From: David Laight @ 2022-02-22 9:09 UTC (permalink / raw)
To: 'Masahiro Yamada'
Cc: linux-kbuild, linux-kernel, Michal Marek, Nick Desaulniers
From: Masahiro Yamada <masahiroy@kernel.org>
> Sent: 22 February 2022 03:44
>
> On Tue, Feb 22, 2022 at 7:33 AM David Laight <David.Laight@aculab.com> wrote:
> >
> > 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.
>
>
> 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.
I was thinking or a slightly more descriptive error message :-)
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-02-22 9:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-21 16:43 [PATCH] fixdep: use fflush() and ferror() to ensure successful write to files Masahiro Yamada
2022-02-21 22:33 ` David Laight
2022-02-22 3:44 ` Masahiro Yamada
2022-02-22 9:09 ` David Laight
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®