Skip to content

Commit

Permalink
clang/win: Fix most -Wwriteable-strings warnings.
Browse files Browse the repository at this point in the history
Many win32 APIs take non-const string pointers. I checked that MSDN documents
them as _In_ and says that they are inputs, and then added const_cast<>s at
the calling sites. (In one test, I introduced a helper struct so that there
can be fewer casts.)

This wasn't just busywork, I found one function that we were handing string
literals where the documentation explicitly said that that's not valid
(filed http://crbug.com/396705).

I didn't change the DECLARE_REGISTRY_APPID_RESOURCEID() call in cloud_print;
it sounds like that'll fix itself when we update to the 2014 sdk:
http://connect.microsoft.com/VisualStudio/feedback/details/806376/atl-hindrances-to-adopting-new-strictstrings-conformance-option-in-vs2013

BUG=396705,82385
R=rnk@chromium.org, rsleevi@chromium.org, sergeyu@chromium.org, vitalybuka@chromium.org

Review URL: https://codereview.chromium.org/413763003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@285051 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
thakis@chromium.org committed Jul 23, 2014
1 parent c8680d6 commit 900634e
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 40 deletions.
2 changes: 1 addition & 1 deletion base/test/test_file_util_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ bool DenyFilePermission(const FilePath& path, DWORD permission) {
change.Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE;
change.Trustee.TrusteeForm = TRUSTEE_IS_NAME;
change.Trustee.TrusteeType = TRUSTEE_IS_USER;
change.Trustee.ptstrName = L"CURRENT_USER";
change.Trustee.ptstrName = const_cast<wchar_t*>(L"CURRENT_USER");

PACL new_dacl;
if (SetEntriesInAcl(1, &change, old_dacl, &new_dacl) != ERROR_SUCCESS) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,8 @@ TEST_F(PortMonitorTest, FlowTest) {
EXPECT_TRUE(monitor2->pfnEndDocPort != NULL);

// These functions should fail if we have not impersonated the user.
EXPECT_FALSE(monitor2->pfnStartDocPort(port_handle, L"", 0, 0, NULL));
EXPECT_FALSE(monitor2->pfnStartDocPort(
port_handle, const_cast<wchar_t*>(L""), 0, 0, NULL));
EXPECT_FALSE(monitor2->pfnWritePort(port_handle,
buffer,
kBufferSize,
Expand All @@ -251,7 +252,8 @@ TEST_F(PortMonitorTest, FlowTest) {

// Now impersonate so we can test the success case.
ASSERT_TRUE(ImpersonateSelf(SecurityImpersonation));
EXPECT_TRUE(monitor2->pfnStartDocPort(port_handle, L"", 0, 0, NULL));
EXPECT_TRUE(monitor2->pfnStartDocPort(
port_handle, const_cast<wchar_t*>(L""), 0, 0, NULL));
EXPECT_TRUE(monitor2->pfnWritePort(port_handle,
buffer,
kBufferSize,
Expand Down
9 changes: 5 additions & 4 deletions net/base/keygen_handler_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ bool GetSubjectPublicKeyInfo(HCRYPTPROV prov, std::vector<BYTE>* output) {
// as a CERT_PUBLIC_KEY_INFO structure. Currently, only RSA public keys are
// supported.
ok = CryptExportPublicKeyInfoEx(prov, AT_KEYEXCHANGE, X509_ASN_ENCODING,
szOID_RSA_RSA, 0, NULL, NULL, &size);
const_cast<char*>(szOID_RSA_RSA), 0, NULL,
NULL, &size);
DCHECK(ok);
if (!ok)
return false;
Expand All @@ -46,8 +47,8 @@ bool GetSubjectPublicKeyInfo(HCRYPTPROV prov, std::vector<BYTE>* output) {
PCERT_PUBLIC_KEY_INFO public_key_casted =
reinterpret_cast<PCERT_PUBLIC_KEY_INFO>(&(*output)[0]);
ok = CryptExportPublicKeyInfoEx(prov, AT_KEYEXCHANGE, X509_ASN_ENCODING,
szOID_RSA_RSA, 0, NULL, public_key_casted,
&size);
const_cast<char*>(szOID_RSA_RSA), 0, NULL,
public_key_casted, &size);
DCHECK(ok);
if (!ok)
return false;
Expand Down Expand Up @@ -82,7 +83,7 @@ bool GetSignedPublicKeyAndChallenge(HCRYPTPROV prov,

CRYPT_ALGORITHM_IDENTIFIER sig_alg;
memset(&sig_alg, 0, sizeof(sig_alg));
sig_alg.pszObjId = szOID_RSA_MD5RSA;
sig_alg.pszObjId = const_cast<char*>(szOID_RSA_MD5RSA);

BOOL ok;
DWORD size = 0;
Expand Down
2 changes: 1 addition & 1 deletion net/cert/cert_verify_proc_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ int CertVerifyProcWin::VerifyInternal(
// We still need to request szOID_SERVER_GATED_CRYPTO and szOID_SGC_NETSCAPE
// today because some certificate chains need them. IE also requests these
// two usages.
static const LPSTR usage[] = {
static const LPCSTR usage[] = {
szOID_PKIX_KP_SERVER_AUTH,
szOID_SERVER_GATED_CRYPTO,
szOID_SGC_NETSCAPE
Expand Down
16 changes: 14 additions & 2 deletions net/proxy/proxy_config_service_win_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,16 @@
namespace net {

TEST(ProxyConfigServiceWinTest, SetFromIEConfig) {
// Like WINHTTP_CURRENT_USER_IE_PROXY_CONFIG, but with const strings.
struct IEProxyConfig {
BOOL auto_detect;
const wchar_t* auto_config_url;
const wchar_t* proxy;
const wchar_t* proxy_bypass;
};
const struct {
// Input.
WINHTTP_CURRENT_USER_IE_PROXY_CONFIG ie_config;
IEProxyConfig ie_config;

// Expected outputs (fields of the ProxyConfig).
bool auto_detect;
Expand Down Expand Up @@ -190,8 +197,13 @@ TEST(ProxyConfigServiceWinTest, SetFromIEConfig) {
};

for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
WINHTTP_CURRENT_USER_IE_PROXY_CONFIG ie_config = {
tests[i].ie_config.auto_detect,
const_cast<wchar_t*>(tests[i].ie_config.auto_config_url),
const_cast<wchar_t*>(tests[i].ie_config.proxy),
const_cast<wchar_t*>(tests[i].ie_config.proxy_bypass)};
ProxyConfig config;
ProxyConfigServiceWin::SetFromIEConfig(&config, tests[i].ie_config);
ProxyConfigServiceWin::SetFromIEConfig(&config, ie_config);

EXPECT_EQ(tests[i].auto_detect, config.auto_detect());
EXPECT_EQ(tests[i].pac_url, config.pac_url());
Expand Down
8 changes: 6 additions & 2 deletions printing/backend/win_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -472,14 +472,18 @@ scoped_ptr<DEVMODE, base::FreeDeleter> CreateDevModeWithColor(

scoped_ptr<DEVMODE, base::FreeDeleter> CreateDevMode(HANDLE printer,
DEVMODE* in) {
LONG buffer_size = DocumentProperties(NULL, printer, L"", NULL, NULL, 0);
LONG buffer_size = DocumentProperties(
NULL, printer, const_cast<wchar_t*>(L""), NULL, NULL, 0);
if (buffer_size < static_cast<int>(sizeof(DEVMODE)))
return scoped_ptr<DEVMODE, base::FreeDeleter>();
scoped_ptr<DEVMODE, base::FreeDeleter> out(
reinterpret_cast<DEVMODE*>(malloc(buffer_size)));
DWORD flags = (in ? (DM_IN_BUFFER) : 0) | DM_OUT_BUFFER;
if (DocumentProperties(NULL, printer, L"", out.get(), in, flags) != IDOK)
if (DocumentProperties(
NULL, printer, const_cast<wchar_t*>(L""), out.get(), in, flags) !=
IDOK) {
return scoped_ptr<DEVMODE, base::FreeDeleter>();
}
CHECK_GE(buffer_size, out.get()->dmSize + out.get()->dmDriverExtra);
return out.Pass();
}
Expand Down
56 changes: 32 additions & 24 deletions remoting/base/dispatch_win.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,14 @@ COMPILE_ASSERT(sizeof(ScopedVariantArg) == sizeof(VARIANTARG),
// - VARIANT is the only supported parameter type at the moment.

HRESULT Invoke(IDispatch* object,
LPOLESTR name,
LPCOLESTR const_name,
WORD flags,
VARIANT* const & result_out) {
// Retrieve the ID of the method to be called.
DISPID disp_id;
HRESULT hr = object->GetIDsOfNames(IID_NULL, &name, 1, LOCALE_USER_DEFAULT,
&disp_id);
LPOLESTR name = const_cast<LPOLESTR>(const_name);
HRESULT hr = object->GetIDsOfNames(
IID_NULL, &name, 1, LOCALE_USER_DEFAULT, &disp_id);
if (FAILED(hr))
return hr;

Expand Down Expand Up @@ -161,14 +162,15 @@ HRESULT Invoke(IDispatch* object,

template <typename P1>
HRESULT Invoke(IDispatch* object,
LPOLESTR name,
LPCOLESTR const_name,
WORD flags,
const P1& p1,
VARIANT* const & result_out) {
// Retrieve the ID of the method to be called.
DISPID disp_id;
HRESULT hr = object->GetIDsOfNames(IID_NULL, &name, 1, LOCALE_USER_DEFAULT,
&disp_id);
LPOLESTR name = const_cast<LPOLESTR>(const_name);
HRESULT hr = object->GetIDsOfNames(
IID_NULL, &name, 1, LOCALE_USER_DEFAULT, &disp_id);
if (FAILED(hr))
return hr;

Expand Down Expand Up @@ -213,15 +215,16 @@ HRESULT Invoke(IDispatch* object,

template <typename P1, typename P2>
HRESULT Invoke(IDispatch* object,
LPOLESTR name,
LPCOLESTR const_name,
WORD flags,
const P1& p1,
const P2& p2,
VARIANT* const & result_out) {
// Retrieve the ID of the method to be called.
DISPID disp_id;
HRESULT hr = object->GetIDsOfNames(IID_NULL, &name, 1, LOCALE_USER_DEFAULT,
&disp_id);
LPOLESTR name = const_cast<LPOLESTR>(const_name);
HRESULT hr = object->GetIDsOfNames(
IID_NULL, &name, 1, LOCALE_USER_DEFAULT, &disp_id);
if (FAILED(hr))
return hr;

Expand Down Expand Up @@ -270,16 +273,17 @@ HRESULT Invoke(IDispatch* object,

template <typename P1, typename P2, typename P3>
HRESULT Invoke(IDispatch* object,
LPOLESTR name,
LPCOLESTR const_name,
WORD flags,
const P1& p1,
const P2& p2,
const P3& p3,
VARIANT* const & result_out) {
// Retrieve the ID of the method to be called.
DISPID disp_id;
HRESULT hr = object->GetIDsOfNames(IID_NULL, &name, 1, LOCALE_USER_DEFAULT,
&disp_id);
LPOLESTR name = const_cast<LPOLESTR>(const_name);
HRESULT hr = object->GetIDsOfNames(
IID_NULL, &name, 1, LOCALE_USER_DEFAULT, &disp_id);
if (FAILED(hr))
return hr;

Expand Down Expand Up @@ -332,7 +336,7 @@ HRESULT Invoke(IDispatch* object,

template <typename P1, typename P2, typename P3, typename P4>
HRESULT Invoke(IDispatch* object,
LPOLESTR name,
LPCOLESTR const_name,
WORD flags,
const P1& p1,
const P2& p2,
Expand All @@ -341,8 +345,9 @@ HRESULT Invoke(IDispatch* object,
VARIANT* const & result_out) {
// Retrieve the ID of the method to be called.
DISPID disp_id;
HRESULT hr = object->GetIDsOfNames(IID_NULL, &name, 1, LOCALE_USER_DEFAULT,
&disp_id);
LPOLESTR name = const_cast<LPOLESTR>(const_name);
HRESULT hr = object->GetIDsOfNames(
IID_NULL, &name, 1, LOCALE_USER_DEFAULT, &disp_id);
if (FAILED(hr))
return hr;

Expand Down Expand Up @@ -399,7 +404,7 @@ HRESULT Invoke(IDispatch* object,

template <typename P1, typename P2, typename P3, typename P4, typename P5>
HRESULT Invoke(IDispatch* object,
LPOLESTR name,
LPCOLESTR const_name,
WORD flags,
const P1& p1,
const P2& p2,
Expand All @@ -409,8 +414,9 @@ HRESULT Invoke(IDispatch* object,
VARIANT* const & result_out) {
// Retrieve the ID of the method to be called.
DISPID disp_id;
HRESULT hr = object->GetIDsOfNames(IID_NULL, &name, 1, LOCALE_USER_DEFAULT,
&disp_id);
LPOLESTR name = const_cast<LPOLESTR>(const_name);
HRESULT hr = object->GetIDsOfNames(
IID_NULL, &name, 1, LOCALE_USER_DEFAULT, &disp_id);
if (FAILED(hr))
return hr;

Expand Down Expand Up @@ -472,7 +478,7 @@ HRESULT Invoke(IDispatch* object,
template <typename P1, typename P2, typename P3, typename P4, typename P5,
typename P6>
HRESULT Invoke(IDispatch* object,
LPOLESTR name,
LPCOLESTR const_name,
WORD flags,
const P1& p1,
const P2& p2,
Expand All @@ -483,8 +489,9 @@ HRESULT Invoke(IDispatch* object,
VARIANT* const & result_out) {
// Retrieve the ID of the method to be called.
DISPID disp_id;
HRESULT hr = object->GetIDsOfNames(IID_NULL, &name, 1, LOCALE_USER_DEFAULT,
&disp_id);
LPOLESTR name = const_cast<LPOLESTR>(const_name);
HRESULT hr = object->GetIDsOfNames(
IID_NULL, &name, 1, LOCALE_USER_DEFAULT, &disp_id);
if (FAILED(hr))
return hr;

Expand Down Expand Up @@ -550,7 +557,7 @@ HRESULT Invoke(IDispatch* object,
template <typename P1, typename P2, typename P3, typename P4, typename P5,
typename P6, typename P7>
HRESULT Invoke(IDispatch* object,
LPOLESTR name,
LPCOLESTR const_name,
WORD flags,
const P1& p1,
const P2& p2,
Expand All @@ -562,8 +569,9 @@ HRESULT Invoke(IDispatch* object,
VARIANT* const & result_out) {
// Retrieve the ID of the method to be called.
DISPID disp_id;
HRESULT hr = object->GetIDsOfNames(IID_NULL, &name, 1, LOCALE_USER_DEFAULT,
&disp_id);
LPOLESTR name = const_cast<LPOLESTR>(const_name);
HRESULT hr = object->GetIDsOfNames(
IID_NULL, &name, 1, LOCALE_USER_DEFAULT, &disp_id);
if (FAILED(hr))
return hr;

Expand Down
7 changes: 4 additions & 3 deletions remoting/base/dispatch_win.h.pump
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ $range ARG 1..ARITY
$if ARITY > 0 [[template <$for ARG , [[typename P$(ARG)]]>]]

HRESULT Invoke(IDispatch* object,
LPOLESTR name,
LPCOLESTR const_name,
WORD flags,
$for ARG [[

Expand All @@ -138,8 +138,9 @@ $for ARG [[
VARIANT* const & result_out) {
// Retrieve the ID of the method to be called.
DISPID disp_id;
HRESULT hr = object->GetIDsOfNames(IID_NULL, &name, 1, LOCALE_USER_DEFAULT,
&disp_id);
LPOLESTR name = const_cast<LPOLESTR>(const_name);
HRESULT hr = object->GetIDsOfNames(
IID_NULL, &name, 1, LOCALE_USER_DEFAULT, &disp_id);
if (FAILED(hr))
return hr;

Expand Down
2 changes: 1 addition & 1 deletion rlz/win/lib/rlz_lib_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ bool CreateMachineState() {
ea.grfAccessMode = GRANT_ACCESS;
ea.grfInheritance= SUB_CONTAINERS_AND_OBJECTS_INHERIT;
ea.Trustee.TrusteeForm = TRUSTEE_IS_NAME;
ea.Trustee.ptstrName = L"Everyone";
ea.Trustee.ptstrName = const_cast<wchar_t*>(L"Everyone");

ACL* new_dacl = NULL;
result = SetEntriesInAcl(1, &ea, dacl, &new_dacl);
Expand Down

0 comments on commit 900634e

Please sign in to comment.