Skip to content

Commit

Permalink
LibC+Kernel: Remove global variable use from snprintf and fprintf
Browse files Browse the repository at this point in the history
The global variable use in these functions is super thread-unsafe and
means that any concurrent calls to sprintf or fprintf in a process
could race with each other and end up writing unexpected results.
We can just replace the function + global variable with a lambda that
captures the relevant argument when calling printf_internal instead.
  • Loading branch information
ADKaster authored and bgianfo committed Feb 9, 2022
1 parent 8ec4328 commit 353e72a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 32 deletions.
22 changes: 10 additions & 12 deletions Kernel/kprintf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,26 +123,24 @@ int sprintf(char* buffer, const char* fmt, ...)
return ret;
}

static size_t __vsnprintf_space_remaining;
ALWAYS_INLINE void sized_buffer_putch(char*& bufptr, char ch)
{
if (__vsnprintf_space_remaining) {
*bufptr++ = ch;
--__vsnprintf_space_remaining;
}
}

int snprintf(char* buffer, size_t size, const char* fmt, ...)
{
va_list ap;
va_start(ap, fmt);
size_t space_remaining = 0;
if (size) {
__vsnprintf_space_remaining = size - 1;
space_remaining = size - 1;
} else {
__vsnprintf_space_remaining = 0;
space_remaining = 0;
}
auto sized_buffer_putch = [&](char*& bufptr, char ch) {
if (space_remaining) {
*bufptr++ = ch;
--space_remaining;
}
};
int ret = printf_internal(sized_buffer_putch, buffer, fmt, ap);
if (__vsnprintf_space_remaining) {
if (space_remaining) {
buffer[ret] = '\0';
} else if (size > 0) {
buffer[size - 1] = '\0';
Expand Down
31 changes: 11 additions & 20 deletions Userland/Libraries/LibC/stdio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -870,17 +870,10 @@ ALWAYS_INLINE void stdout_putch(char*&, char ch)
putchar(ch);
}

static FILE* __current_stream = nullptr;
ALWAYS_INLINE static void stream_putch(char*&, char ch)
{
fputc(ch, __current_stream);
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/vfprintf.html
int vfprintf(FILE* stream, const char* fmt, va_list ap)
{
__current_stream = stream;
return printf_internal(stream_putch, nullptr, fmt, ap);
return printf_internal([stream](auto, char ch) { fputc(ch, stream); }, nullptr, fmt, ap);
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/fprintf.html
Expand Down Expand Up @@ -957,25 +950,23 @@ int sprintf(char* buffer, const char* fmt, ...)
return ret;
}

static size_t __vsnprintf_space_remaining;
ALWAYS_INLINE void sized_buffer_putch(char*& bufptr, char ch)
{
if (__vsnprintf_space_remaining) {
*bufptr++ = ch;
--__vsnprintf_space_remaining;
}
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/vsnprintf.html
int vsnprintf(char* buffer, size_t size, const char* fmt, va_list ap)
{
size_t space_remaining = 0;
if (size) {
__vsnprintf_space_remaining = size - 1;
space_remaining = size - 1;
} else {
__vsnprintf_space_remaining = 0;
space_remaining = 0;
}
auto sized_buffer_putch = [&](char*& bufptr, char ch) {
if (space_remaining) {
*bufptr++ = ch;
--space_remaining;
}
};
int ret = printf_internal(sized_buffer_putch, buffer, fmt, ap);
if (__vsnprintf_space_remaining) {
if (space_remaining) {
buffer[ret] = '\0';
} else if (size > 0) {
buffer[size - 1] = '\0';
Expand Down

0 comments on commit 353e72a

Please sign in to comment.