* Re: conf: flush stdout before early exit
2025-05-29 17:08 conf: flush stdout before early exit Jon Maloy
@ 2025-05-30 2:21 ` David Gibson
2025-05-30 8:08 ` Stefano Brivio
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2025-05-30 2:21 UTC (permalink / raw)
To: Jon Maloy; +Cc: sbrivio, dgibson, passt-dev
[-- Attachment #1: Type: text/plain, Size: 1732 bytes --]
On Thu, May 29, 2025 at 01:08:58PM -0400, Jon Maloy wrote:
> Before doing an early exit any contents of stdout is normally flushed.
> This doesn't happen when the output goes into a pipe and we return with
> _exit(). We now add an explicit flush in such cases.
Minor nitpicking on this description. AIUI
* We'll *never* flush on exit with _exit(), not only when the output
is a pipe
* The reason we don't see the problem when stdout is a terminal is
that in that case stdio flushes the buffer on each line
> Fixes: d0006fa784a7 ("treewide: use _exit() over exit()")
>
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
> conf.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/conf.c b/conf.c
> index a6d7e22..a1d55a3 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -975,6 +975,7 @@ static void usage(const char *name, FILE *f, int status)
> " SPEC is as described for TCP above\n"
> " default: none\n");
>
> + fflush(f);
> _exit(status);
>
> pasta_opts:
> @@ -1029,6 +1030,7 @@ pasta_opts:
> " --ns-mac-addr ADDR Set MAC address on tap interface\n"
> " --no-splice Disable inbound socket splicing\n");
>
> + fflush(f);
> _exit(status);
> }
>
> @@ -1594,6 +1596,7 @@ void conf(struct ctx *c, int argc, char **argv)
> FPRINTF(stdout,
> c->mode == MODE_PASTA ? "pasta " : "passt ");
> FPRINTF(stdout, VERSION_BLOB);
> + fflush(stdout);
> _exit(EXIT_SUCCESS);
> case 15:
> ret = snprintf(c->ip4.ifname_out,
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: conf: flush stdout before early exit
2025-05-29 17:08 conf: flush stdout before early exit Jon Maloy
2025-05-30 2:21 ` David Gibson
@ 2025-05-30 8:08 ` Stefano Brivio
2025-05-30 10:00 ` Paul Holzinger
2025-06-04 15:00 ` Stefano Brivio
3 siblings, 0 replies; 6+ messages in thread
From: Stefano Brivio @ 2025-05-30 8:08 UTC (permalink / raw)
To: Jon Maloy; +Cc: dgibson, passt-dev, Paul Holzinger, John Radley (jradxl2)
Thanks for the patch! Cc'ing with full quote for review Paul as author
of d0006fa784a7, and John as reporter of the issue.
On Thu, 29 May 2025 13:08:58 -0400
Jon Maloy <jmaloy@redhat.com> wrote:
> Before doing an early exit any contents of stdout is normally flushed.
> This doesn't happen when the output goes into a pipe and we return with
> _exit(). We now add an explicit flush in such cases.
>
> Fixes: d0006fa784a7 ("treewide: use _exit() over exit()")
>
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
> conf.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/conf.c b/conf.c
> index a6d7e22..a1d55a3 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -975,6 +975,7 @@ static void usage(const char *name, FILE *f, int status)
> " SPEC is as described for TCP above\n"
> " default: none\n");
>
> + fflush(f);
> _exit(status);
>
> pasta_opts:
> @@ -1029,6 +1030,7 @@ pasta_opts:
> " --ns-mac-addr ADDR Set MAC address on tap interface\n"
> " --no-splice Disable inbound socket splicing\n");
>
> + fflush(f);
> _exit(status);
> }
>
> @@ -1594,6 +1596,7 @@ void conf(struct ctx *c, int argc, char **argv)
> FPRINTF(stdout,
> c->mode == MODE_PASTA ? "pasta " : "passt ");
> FPRINTF(stdout, VERSION_BLOB);
> + fflush(stdout);
> _exit(EXIT_SUCCESS);
> case 15:
> ret = snprintf(c->ip4.ifname_out,
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: conf: flush stdout before early exit
2025-05-29 17:08 conf: flush stdout before early exit Jon Maloy
2025-05-30 2:21 ` David Gibson
2025-05-30 8:08 ` Stefano Brivio
@ 2025-05-30 10:00 ` Paul Holzinger
2025-06-04 15:00 ` Stefano Brivio
3 siblings, 0 replies; 6+ messages in thread
From: Paul Holzinger @ 2025-05-30 10:00 UTC (permalink / raw)
To: Jon Maloy, sbrivio, dgibson, passt-dev
Hi,
On 29/05/2025 19:08, Jon Maloy wrote:
> Before doing an early exit any contents of stdout is normally flushed.
> This doesn't happen when the output goes into a pipe and we return with
> _exit(). We now add an explicit flush in such cases.
>
> Fixes: d0006fa784a7 ("treewide: use _exit() over exit()")
>
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
Reviewed-by: Paul Holzinger <pholzing@redhat.com>
Interestingly enough I never noticed that I broke podman info with this:
podman info --format "{{.Host.Pasta.Version}}"
The output is empty currently due this issue and works with this patch
again.
> ---
> conf.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/conf.c b/conf.c
> index a6d7e22..a1d55a3 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -975,6 +975,7 @@ static void usage(const char *name, FILE *f, int status)
> " SPEC is as described for TCP above\n"
> " default: none\n");
>
> + fflush(f);
> _exit(status);
>
> pasta_opts:
> @@ -1029,6 +1030,7 @@ pasta_opts:
> " --ns-mac-addr ADDR Set MAC address on tap interface\n"
> " --no-splice Disable inbound socket splicing\n");
>
> + fflush(f);
> _exit(status);
> }
>
> @@ -1594,6 +1596,7 @@ void conf(struct ctx *c, int argc, char **argv)
> FPRINTF(stdout,
> c->mode == MODE_PASTA ? "pasta " : "passt ");
> FPRINTF(stdout, VERSION_BLOB);
> + fflush(stdout);
> _exit(EXIT_SUCCESS);
> case 15:
> ret = snprintf(c->ip4.ifname_out,
--
Paul Holzinger
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: conf: flush stdout before early exit
2025-05-29 17:08 conf: flush stdout before early exit Jon Maloy
` (2 preceding siblings ...)
2025-05-30 10:00 ` Paul Holzinger
@ 2025-06-04 15:00 ` Stefano Brivio
2025-06-06 10:06 ` Stefano Brivio
3 siblings, 1 reply; 6+ messages in thread
From: Stefano Brivio @ 2025-06-04 15:00 UTC (permalink / raw)
To: Jon Maloy; +Cc: dgibson, passt-dev
On Thu, 29 May 2025 13:08:58 -0400
Jon Maloy <jmaloy@redhat.com> wrote:
> Before doing an early exit any contents of stdout is normally flushed.
> This doesn't happen when the output goes into a pipe and we return with
> _exit(). We now add an explicit flush in such cases.
>
> Fixes: d0006fa784a7 ("treewide: use _exit() over exit()")
>
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
> conf.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/conf.c b/conf.c
> index a6d7e22..a1d55a3 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -975,6 +975,7 @@ static void usage(const char *name, FILE *f, int status)
> " SPEC is as described for TCP above\n"
> " default: none\n");
>
> + fflush(f);
> _exit(status);
>
> pasta_opts:
> @@ -1029,6 +1030,7 @@ pasta_opts:
> " --ns-mac-addr ADDR Set MAC address on tap interface\n"
> " --no-splice Disable inbound socket splicing\n");
>
> + fflush(f);
> _exit(status);
> }
>
> @@ -1594,6 +1596,7 @@ void conf(struct ctx *c, int argc, char **argv)
> FPRINTF(stdout,
> c->mode == MODE_PASTA ? "pasta " : "passt ");
> FPRINTF(stdout, VERSION_BLOB);
> + fflush(stdout);
I was about to apply this, but clang-tidy now complains:
/home/sbrivio/passt/conf.c:978:2: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [cert-err33-c,-warnings-as-errors]
978 | fflush(f);
| ^~~~~~~~~
/home/sbrivio/passt/conf.c:978:2: note: cast the expression to void to silence this warning
/home/sbrivio/passt/conf.c:1033:2: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [cert-err33-c,-warnings-as-errors]
1033 | fflush(f);
| ^~~~~~~~~
/home/sbrivio/passt/conf.c:1033:2: note: cast the expression to void to silence this warning
/home/sbrivio/passt/conf.c:1599:4: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [cert-err33-c,-warnings-as-errors]
1599 | fflush(stdout);
| ^~~~~~~~~~~~~~
/home/sbrivio/passt/conf.c:1599:4: note: cast the expression to void to silence this warning
I don't see any sensible way to use the return value from fflush(),
because, of course, if 'f' or stdout is not a valid stream or isn't open
for writing (that's the only defined error for fflush()), it doesn't
make sense to even try and print a warning to stdout or stderr.
So I would rather suppress the warning as suggested by clang-tidy
(casting to void instead of a specific clang-tidy suppression, just in
case other static checkers start complaining as well). I can do this on
merge if you're fine with it.
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: conf: flush stdout before early exit
2025-06-04 15:00 ` Stefano Brivio
@ 2025-06-06 10:06 ` Stefano Brivio
0 siblings, 0 replies; 6+ messages in thread
From: Stefano Brivio @ 2025-06-06 10:06 UTC (permalink / raw)
To: Jon Maloy; +Cc: dgibson, passt-dev
On Wed, 4 Jun 2025 17:00:36 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:
> On Thu, 29 May 2025 13:08:58 -0400
> Jon Maloy <jmaloy@redhat.com> wrote:
>
> > Before doing an early exit any contents of stdout is normally flushed.
> > This doesn't happen when the output goes into a pipe and we return with
> > _exit(). We now add an explicit flush in such cases.
> >
> > Fixes: d0006fa784a7 ("treewide: use _exit() over exit()")
> >
> > Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> > ---
> > conf.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/conf.c b/conf.c
> > index a6d7e22..a1d55a3 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -975,6 +975,7 @@ static void usage(const char *name, FILE *f, int status)
> > " SPEC is as described for TCP above\n"
> > " default: none\n");
> >
> > + fflush(f);
> > _exit(status);
> >
> > pasta_opts:
> > @@ -1029,6 +1030,7 @@ pasta_opts:
> > " --ns-mac-addr ADDR Set MAC address on tap interface\n"
> > " --no-splice Disable inbound socket splicing\n");
> >
> > + fflush(f);
> > _exit(status);
> > }
> >
> > @@ -1594,6 +1596,7 @@ void conf(struct ctx *c, int argc, char **argv)
> > FPRINTF(stdout,
> > c->mode == MODE_PASTA ? "pasta " : "passt ");
> > FPRINTF(stdout, VERSION_BLOB);
> > + fflush(stdout);
>
> I was about to apply this, but clang-tidy now complains:
>
> /home/sbrivio/passt/conf.c:978:2: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [cert-err33-c,-warnings-as-errors]
> 978 | fflush(f);
> | ^~~~~~~~~
> /home/sbrivio/passt/conf.c:978:2: note: cast the expression to void to silence this warning
> /home/sbrivio/passt/conf.c:1033:2: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [cert-err33-c,-warnings-as-errors]
> 1033 | fflush(f);
> | ^~~~~~~~~
> /home/sbrivio/passt/conf.c:1033:2: note: cast the expression to void to silence this warning
> /home/sbrivio/passt/conf.c:1599:4: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [cert-err33-c,-warnings-as-errors]
> 1599 | fflush(stdout);
> | ^~~~~~~~~~~~~~
> /home/sbrivio/passt/conf.c:1599:4: note: cast the expression to void to silence this warning
>
> I don't see any sensible way to use the return value from fflush(),
> because, of course, if 'f' or stdout is not a valid stream or isn't open
> for writing (that's the only defined error for fflush()), it doesn't
> make sense to even try and print a warning to stdout or stderr.
>
> So I would rather suppress the warning as suggested by clang-tidy
> (casting to void instead of a specific clang-tidy suppression, just in
> case other static checkers start complaining as well). I can do this on
> merge if you're fine with it.
I took that as a yes and went ahead with it, thanks for the fix!
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread