Skip to content

Flush output buffers proactively#201

Merged
nickdesaulniers merged 1 commit into
ClangBuiltLinux:mainfrom
kees:flush
Sep 22, 2021
Merged

Flush output buffers proactively#201
nickdesaulniers merged 1 commit into
ClangBuiltLinux:mainfrom
kees:flush

Conversation

@kees

@kees kees commented Sep 18, 2021

Copy link
Copy Markdown
Contributor

In the CI logs, QEMU output shows up before the rest of check_log.py
output. This is likely due to IO buffering, so explicitly flush buffers
in the helper routines and before spawning QEMU.

Signed-off-by: Kees Cook keescook@google.com

Comment thread utils.py

def print_red(msg):
print("\033[91m%s\033[0m" % msg, file=sys.stderr)
sys.stderr.flush()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stderr is unbuffered and many applications leverage this property. No need for flush.

@nickdesaulniers nickdesaulniers Sep 20, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, see also man 3 stderr:

The stream stderr is unbuffered.

TIL!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though note that it seems that @kees is modifying the buffering of stderr for our boot wrapper script in ClangBuiltLinux/boot-utils#49.

Comment thread utils.py

def print_red(msg):
print("\033[91m%s\033[0m" % msg, file=sys.stderr)
sys.stderr.flush()

@nickdesaulniers nickdesaulniers Sep 20, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, see also man 3 stderr:

The stream stderr is unbuffered.

TIL!

@kees

kees commented Sep 22, 2021

Copy link
Copy Markdown
Contributor Author

What changes are desired here? It's not harmful to flush stderr. At best it fixes intermixed output, and at worst it is redundant. :)

@nickdesaulniers

Copy link
Copy Markdown
Member

Sure, but at the least the branch needs to be rebased on main and force pushed before we can merge.

In the CI logs, QEMU output shows up before the rest of check_log.py
output. This is likely due to IO buffering, so explicitly flush buffers
in the helper routines and before spawning QEMU.

Signed-off-by: Kees Cook <keescook@google.com>
@kees

kees commented Sep 22, 2021

Copy link
Copy Markdown
Contributor Author

Okay, rebased. :)

@nickdesaulniers nickdesaulniers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't matter to me

@nickdesaulniers nickdesaulniers merged commit 25548eb into ClangBuiltLinux:main Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants