AuroraMiddleware/winpty
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Replace c99_[v]snprintf with winpty_[v]snprintf and improve trace() checks

Browse Source 
The existing snprintf functions were not C99-conformant:

 - The _vsnprintf* functions do not accept all of the C99 specifiers,
   like %zu for size_t or (on WinXP) %lld for long long.

 - MinGW always provides vsnprintf, but the implementation sometimes just
   calls MSVCRT's _vsnprintf*.  In that case, it won't handle all C99
   specifiers and the return value won't be conformant.

Changes:

 - winpty_snprintf accepts C90(?) specifiers, returns -1 on error
   (including truncation) and should work with the inttypes.h printf
   macros.

 - Enable __attribute__((printf)) on the winpty_[v]snprintf functions and
   also on trace().  Fix a HANDLE(void*) <-> unsigned int mismatch in
   NamedPipe.cc.

 - Switch most sprintf() call sites to winpty_snprintf for improved safety
   guarantees.  (This change gets the code closer to compiling with MSVC's
   /sdl flag, which forbids "unsafe" memory/string functions.)
This commit is contained in:
Ryan Prichard 2016-03-02 02:35:50 -06:00
parent adc1a5adbf
commit 6c7eac63b8
15 changed files with 137 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/agent/Agent.cc
View File

@ -27,7 +27,7 @@
#include "../shared/DebugClient.h"
#include "../shared/AgentMsg.h"
#include "../shared/Buffer.h"
#include "../shared/c99_snprintf.h"
#include "../shared/winpty_snprintf.h"
#include "../shared/WinptyAssert.h"
#include <stdio.h>
#include <stdlib.h>
@ -41,8 +41,6 @@
const int SC_CONSOLE_MARK = 0xFFF2;
const int SC_CONSOLE_SELECT_ALL = 0xFFF5;
#define COUNT_OF(x) (sizeof(x) / sizeof((x)[0]))
namespace {
static BOOL WINAPI consoleCtrlHandler(DWORD dwCtrlType)
@ -836,7 +834,7 @@ void Agent::syncMarkerText(CHAR_INFO (&output)[SYNC_MARKER_LEN])
// XXX: The marker text generated here could easily collide with ordinary
// console output. Does it make sense to try to avoid the collision?
char str[SYNC_MARKER_LEN];
c99_snprintf(str, COUNT_OF(str), "S*Y*N*C*%08x", m_syncCounter);
winpty_snprintf(str, "S*Y*N*C*%08x", m_syncCounter);
for (int i = 0; i < SYNC_MARKER_LEN; ++i) {
output[i].Char.UnicodeChar = str[i];
output[i].Attributes = 7;

8
src/agent/ConsoleFont.cc
View File

@ -32,6 +32,7 @@
#include "../shared/DebugClient.h"
#include "../shared/OsModule.h"
#include "../shared/WinptyAssert.h"
#include "../shared/winpty_snprintf.h"
#include "../shared/winpty_wcsnlen.h"
namespace {
@ -298,14 +299,15 @@ static void dumpFontTable(HANDLE conout, const char *prefix) {
size_t first = 0;
while (first < table.size()) {
size_t last = std::min(table.size() - 1, first + 10 - 1);
sprintf(tmp, "%sfonts %02u-%02u:",
winpty_snprintf(tmp, "%sfonts %02u-%02u:",
prefix, static_cast<unsigned>(first), static_cast<unsigned>(last));
line = tmp;
for (size_t i = first; i <= last; ++i) {
if (i % 10 == 5) {
line += " - ";
}
sprintf(tmp, " %2dx%-2d", table[i].second.X, table[i].second.Y);
winpty_snprintf(tmp, " %2dx%-2d",
table[i].second.X, table[i].second.Y);
line += tmp;
}
trace("%s", line.c_str());
@ -336,7 +338,7 @@ static std::string stringToCodePoints(const std::wstring &str) {
std::string ret = "(";
for (size_t i = 0; i < str.size(); ++i) {
char tmp[32];
sprintf(tmp, "%X", str[i]);
winpty_snprintf(tmp, "%X", str[i]);
if (ret.size() > 1) {
ret.push_back(' ');
}

2
src/agent/ConsoleInput.cc
View File

@ -256,7 +256,7 @@ void ConsoleInput::writeInput(const std::string &input)
}
const unsigned char uch = input[i];
char buf[32];
sprintf(buf, "%02X", uch);
winpty_snprintf(buf, "%02X", uch);
dumpString += buf;
}
dumpString += ')';

5
src/agent/Coord.cc
View File

@ -19,11 +19,14 @@
// IN THE SOFTWARE.
#include "Coord.h"
#include <stdio.h>
#include "../shared/winpty_snprintf.h"
std::string Coord::toString() const
{
char ret[32];
sprintf(ret, "(%d,%d)", X, Y);
winpty_snprintf(ret, "(%d,%d)", X, Y);
return std::string(ret);
}

7
src/agent/InputMap.cc
View File

@ -30,6 +30,7 @@
#include "../shared/DebugClient.h"
#include "../shared/UnixCtrlChars.h"
#include "../shared/WinptyAssert.h"
#include "../shared/winpty_snprintf.h"
namespace {
@ -122,13 +123,13 @@ std::string InputMap::Key::toString() const {
(virtualKey >= '0' && virtualKey <= '9')) {
ret += static_cast<char>(virtualKey);
} else {
sprintf(buf, "0x%x", virtualKey);
winpty_snprintf(buf, "0x%x", virtualKey);
ret += buf;
}
if (unicodeChar >= 32 && unicodeChar <= 126) {
sprintf(buf, " ch='%c'", unicodeChar);
winpty_snprintf(buf, " ch='%c'", unicodeChar);
} else {
sprintf(buf, " ch=%#x", unicodeChar);
winpty_snprintf(buf, " ch=%#x", unicodeChar);
}
ret += buf;
return ret;

2
src/agent/NamedPipe.cc
View File

@ -190,7 +190,7 @@ bool NamedPipe::connectToServer(LPCWSTR pipeName)
OPEN_EXISTING,
FILE_FLAG_OVERLAPPED,
NULL);
trace("connection to [%ls], handle == 0x%x", pipeName, handle);
trace("connection to [%ls], handle == %p", pipeName, handle);
if (handle == INVALID_HANDLE_VALUE)
return false;
m_handle = handle;

5
src/agent/SmallRect.cc
View File

@ -19,12 +19,15 @@
// IN THE SOFTWARE.
#include "SmallRect.h"
#include <stdio.h>
#include "../shared/winpty_snprintf.h"
std::string SmallRect::toString() const
{
char ret[64];
sprintf(ret, "(x=%d,y=%d,w=%d,h=%d)",
winpty_snprintf(ret, "(x=%d,y=%d,w=%d,h=%d)",
Left, Top, width(), height());
return std::string(ret);
}

6
src/agent/Terminal.cc
View File

@ -30,6 +30,7 @@
#include "UnicodeEncoding.h"
#include "../shared/DebugClient.h"
#include "../shared/WinptyAssert.h"
#include "../shared/winpty_snprintf.h"
#define CSI "\x1b["
@ -369,7 +370,7 @@ void Terminal::finishOutput(const std::pair<int, int64_t> &newCursorPos)
if (m_cursorHidden) {
moveTerminalToLine(newCursorPos.second);
char buffer[32];
sprintf(buffer, CSI"%dG" CSI"?25h", newCursorPos.first + 1);
winpty_snprintf(buffer, CSI"%dG" CSI"?25h", newCursorPos.first + 1);
if (!m_consoleMode)
m_output->write(buffer);
m_cursorHidden = false;
@ -396,7 +397,8 @@ void Terminal::moveTerminalToLine(int64_t line)
if (line < m_remoteLine) {
// CUrsor Up (CUU)
char buffer[32];
sprintf(buffer, "\r" CSI"%dA", static_cast<int>(m_remoteLine - line));
winpty_snprintf(buffer, "\r" CSI"%dA",
static_cast<int>(m_remoteLine - line));
if (!m_consoleMode)
m_output->write(buffer);
m_remoteLine = line;

3
src/libwinpty/subdir.mk
View File

@ -24,7 +24,8 @@ $(eval $(call def_mingw_target,libwinpty,-DCOMPILING_WINPTY_DLL))
LIBWINPTY_OBJECTS = \
build/libwinpty/libwinpty/winpty.o \
build/libwinpty/shared/DebugClient.o
build/libwinpty/shared/DebugClient.o \
build/libwinpty/shared/WinptyAssert.o
build/winpty.dll : $(LIBWINPTY_OBJECTS)
$(info Linking $@)

6
src/shared/DebugClient.cc
View File

@ -26,7 +26,7 @@
#include <string>
#include "c99_snprintf.h"
#include "winpty_snprintf.h"
void *volatile g_debugConfig;
@ -111,7 +111,7 @@ void trace(const char *format, ...)
va_list ap;
va_start(ap, format);
c99_vsnprintf(message, sizeof(message), format, ap);
winpty_vsnprintf(message, format, ap);
message[sizeof(message) - 1] = '\0';
va_end(ap);
@ -124,7 +124,7 @@ void trace(const char *format, ...)
baseName = (baseName != NULL) ? baseName + 1 : moduleName;
char fullMessage[1024];
c99_snprintf(fullMessage, sizeof(fullMessage),
winpty_snprintf(fullMessage,
"[%05d.%03d %s,p%04d,t%04d]: %s",
currentTime / 1000, currentTime % 1000,
baseName, (int)GetCurrentProcessId(), (int)GetCurrentThreadId(),

4
src/shared/DebugClient.h
View File

@ -21,8 +21,10 @@
#ifndef DEBUGCLIENT_H
#define DEBUGCLIENT_H
#include "winpty_snprintf.h"
bool isTracingEnabled();
bool hasDebugFlag(const char *flag);
void trace(const char *format, ...);
void trace(const char *format, ...) WINPTY_SNPRINTF_FORMAT(1, 2);
#endif // DEBUGCLIENT_H

92
src/shared/c99_snprintf.h
View File

@ -1,92 +0,0 @@
// Copyright (c) 2012 Ryan Prichard
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to
// deal in the Software without restriction, including without limitation the
// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
// sell copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
// IN THE SOFTWARE.
#ifndef C99_SNPRINTF_H
#define C99_SNPRINTF_H
#include <stdarg.h>
#include <stdio.h>
#ifdef _MSC_VER
#if _MSC_VER < 1800
/* MSVC does not define C99's va_copy, so define one for it. It appears that
* with MSVC, a va_list is a char*, not an array type, so va_copy is a simple
* assignment. On MSVC 2012, there is a VC/include/vadefs.h file defining
* va_list and the va_XXX routines, which has code for x86, x86-64, and ARM. */
#define c99_va_copy(dest, src) ((dest) = (src))
#else
/* Visual C++ 2013 added va_copy. */
#define c99_va_copy(dest, src) va_copy(dest, src)
#endif
static inline int c99_vsnprintf(
char* str,
size_t size,
const char* format,
va_list ap)
{
va_list apcopy;
int count = -1;
if (size != 0) {
c99_va_copy(apcopy, ap);
count = _vsnprintf_s(str, size, _TRUNCATE, format, apcopy);
va_end(apcopy);
}
if (count == -1) {
c99_va_copy(apcopy, ap);
count = _vscprintf(format, apcopy);
va_end(apcopy);
}
return count;
}
#else
#define c99_va_copy(dest, src) (va_copy(dest, src))
static inline int c99_vsnprintf(
char* str,
size_t size,
const char* format,
va_list ap)
{
return vsnprintf(str, size, format, ap);
}
#endif /* _MSC_VER */
static inline int c99_snprintf(
char* str,
size_t size,
const char* format,
...)
{
int count;
va_list ap;
va_start(ap, format);
count = c99_vsnprintf(str, size, format, ap);
va_end(ap);
return count;
}
#endif /* C99_SNPRINTF_H */

99
src/shared/winpty_snprintf.h Normal file
View File

@ -0,0 +1,99 @@
// Copyright (c) 2016 Ryan Prichard
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to
// deal in the Software without restriction, including without limitation the
// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
// sell copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
// IN THE SOFTWARE.
#ifndef WINPTY_SNPRINTF_H
#define WINPTY_SNPRINTF_H
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include "WinptyAssert.h"
#if defined(__CYGWIN__) || defined(__MSYS__)
#define WINPTY_SNPRINTF_FORMAT(fmtarg, vararg) \
__attribute__((format(printf, (fmtarg), ((vararg)))))
#elif defined(__GNUC__)
#define WINPTY_SNPRINTF_FORMAT(fmtarg, vararg) \
__attribute__((format(ms_printf, (fmtarg), ((vararg)))))
#else
#define WINPTY_SNPRINTF_FORMAT(fmtarg, vararg)
#endif
// Returns a value between 0 and size - 1 (inclusive) on success. Returns -1
// on failure (including truncation). The output buffer is always
// NUL-terminated.
inline int
winpty_vsnprintf(char *out, size_t size, const char *fmt, va_list ap) {
ASSERT(size > 0);
out[0] = '\0';
#if defined(_MSC_VER) && _MSC_VER < 1900
// MSVC 2015 added a C99-conforming vsnprintf.
int count = _vsnprintf_s(out, size, _TRUNCATE, fmt, ap);
#else
// MinGW configurations frequently provide a vsnprintf function that simply
// calls one of the MS _vsnprintf* functions, which are not C99 conformant.
int count = vsnprintf(out, size, fmt, ap);
#endif
if (count < 0 || static_cast<size_t>(count) >= size) {
// On truncation, some *printf* implementations return the
// non-truncated size, but other implementations returns -1. Return
// -1 for consistency.
count = -1;
// Guarantee NUL termination.
out[size - 1] = '\0';
} else {
// Guarantee NUL termination.
out[count] = '\0';
}
return count;
}
// Wraps winpty_vsnprintf.
inline int winpty_snprintf(char *out, size_t size, const char *fmt, ...)
WINPTY_SNPRINTF_FORMAT(3, 4);
inline int winpty_snprintf(char *out, size_t size, const char *fmt, ...) {
va_list ap;
va_start(ap, fmt);
const int count = winpty_vsnprintf(out, size, fmt, ap);
va_end(ap);
return count;
}
// Wraps winpty_vsnprintf with automatic size determination.
template <size_t size>
int winpty_vsnprintf(char (&out)[size], const char *fmt, va_list ap) {
return winpty_vsnprintf(out, size, fmt, ap);
}
// Wraps winpty_vsnprintf with automatic size determination.
template <size_t size>
int winpty_snprintf(char (&out)[size], const char *fmt, ...)
WINPTY_SNPRINTF_FORMAT(2, 3);
template <size_t size>
int winpty_snprintf(char (&out)[size], const char *fmt, ...) {
va_list ap;
va_start(ap, fmt);
const int count = winpty_vsnprintf(out, size, fmt, ap);
va_end(ap);
return count;
}
#endif // WINPTY_SNPRINTF_H

1
src/unix-adapter/subdir.mk
View File

@ -29,6 +29,7 @@ UNIX_ADAPTER_OBJECTS = \
build/unix-adapter/unix-adapter/WakeupFd.o \
build/unix-adapter/unix-adapter/main.o \
build/unix-adapter/shared/DebugClient.o \
build/unix-adapter/shared/WinptyAssert.o \
build/unix-adapter/shared/WinptyVersion.o
build/$(UNIX_ADAPTER_EXE) : $(UNIX_ADAPTER_OBJECTS) build/winpty.dll

6
src/winpty.gyp
View File

@ -83,7 +83,7 @@
'shared/WinptyAssert.cc',
'shared/WinptyVersion.h',
'shared/WinptyVersion.cc',
'shared/c99_snprintf.h',
'shared/winpty_snprintf.h',
'shared/winpty_wcsnlen.cc',
'shared/winpty_wcsnlen.h',
],
@ -107,7 +107,9 @@
'shared/Buffer.h',
'shared/DebugClient.h',
'shared/DebugClient.cc',
'shared/c99_snprintf.h',
'shared/WinptyAssert.h',
'shared/WinptyAssert.cc',
'shared/winpty_snprintf.h',
],
},
{