Skip to content

[libc] Lock the output stream for the 'puts' call #76513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 2, 2024
Merged

[libc] Lock the output stream for the 'puts' call #76513

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a57da1
[libc] Lock the output stream for the 'puts' call
jhuber6 Dec 28, 2023
164048b
Use unlocked
jhuber6 Jan 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions libc/src/stdio/generic/puts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,34 @@

namespace LIBC_NAMESPACE {

namespace {

// Simple helper to unlock the file once destroyed.
struct ScopedLock {
ScopedLock(LIBC_NAMESPACE::File *stream) : stream(stream) { stream->lock(); }
~ScopedLock() { stream->unlock(); }

private:
LIBC_NAMESPACE::File *stream;
};

} // namespace

LLVM_LIBC_FUNCTION(int, puts, (const char *__restrict str)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why this is marked as __restrict?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think all of the implementations of string functions are like this. My best guess is that it's giving the same effect as const char const * without changing the signature or something.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, can you please provide more info on this point; it doesn't sound right to me.

My understanding of __restrict is that it has to do with strict aliasing of function parameters. If there's only 1 parameter, there is nothing to be concerned with wrt to aliasing.

So why add __restrict here?

cpp::string_view str_view(str);
auto result = LIBC_NAMESPACE::stdout->write(str, str_view.size());

// We need to lock the stream to ensure the newline is always appended.
ScopedLock lock(LIBC_NAMESPACE::stdout);

auto result = LIBC_NAMESPACE::stdout->write_unlocked(str, str_view.size());
if (result.has_error())
libc_errno = result.error;
size_t written = result.value;
if (str_view.size() != written) {
// The stream should be in an error state in this case.
return EOF;
}
result = LIBC_NAMESPACE::stdout->write("\n", 1);
result = LIBC_NAMESPACE::stdout->write_unlocked("\n", 1);
if (result.has_error())
libc_errno = result.error;
written = result.value;
Expand Down