Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Side by Side Diff: src/plugin/PluginClientBase.cpp

Issue 6012307226230784: Issue #1234 - std::wstring version of UnescapeUrl (Closed)
Patch Set: Created Aug. 5, 2014, 5:51 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « src/plugin/PluginClientBase.h ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 #include "PluginStdAfx.h" 1 #include "PluginStdAfx.h"
2 2
3 // Internet / FTP 3 // Internet / FTP
4 #include <wininet.h> 4 #include <wininet.h>
5 5
6 // IP adapter 6 // IP adapter
7 #include <iphlpapi.h> 7 #include <iphlpapi.h>
8 8
9 #include "PluginSettings.h" 9 #include "PluginSettings.h"
10 #include "PluginSystem.h" 10 #include "PluginSystem.h"
(...skipping 48 matching lines...) Expand 10 before | Expand all | Expand 10 after
59 unescapedUrl.ReleaseBuffer(); 59 unescapedUrl.ReleaseBuffer();
60 unescapedUrl.Truncate(cb); 60 unescapedUrl.Truncate(cb);
61 61
62 url.ReleaseBuffer(); 62 url.ReleaseBuffer();
63 url = unescapedUrl; 63 url = unescapedUrl;
64 } 64 }
65 65
66 return url; 66 return url;
67 } 67 }
68 68
69 void UnescapeUrl(std::wstring& url)
70 {
71 /*
72 * When the code can tolerate exceptions better,
Felix Dahlke 2014/08/15 15:32:29 I think this comment should go into the catch bloc
Eric 2014/09/29 11:50:07 OK. Since I've written this code, I've come to th
73 * there's no reason to keep this try-catch statement in place.
74 */
75 try
76 {
77 DWORD result_length = INTERNET_MAX_URL_LENGTH;
78 /*
79 * The buffer length is greater than 2 Kb, so we keep it off the stack and a llocate it.
Eric 2014/09/29 11:50:07 Done.
80 */
81 std::unique_ptr<wchar_t[]> result(new wchar_t[result_length]); // can throw bad_alloc
82 /*
83 * Casting away const here is harmless because we're not using the in-place modification mode of UrlUnescape
Felix Dahlke 2014/08/15 15:32:29 This is pretty obvious from the documentation of U
Felix Dahlke 2014/09/30 09:10:28 We have the exact same problem in several cases -
Eric 2014/09/30 15:22:21 UrlUnescape is worse than the usual Windows API, b
Felix Dahlke 2014/09/30 16:08:16 Yeah, that's true. Still - at the end of the day t
sergei 2014/10/06 09:08:54 I would say that this comment could be here or may
84 */
85 HRESULT hr = UrlUnescapeW(const_cast<wchar_t *>(url.c_str()), result.get(), &result_length, 0);
Felix Dahlke 2014/08/15 15:32:29 No space between wchar_t and *.
Eric 2014/09/29 11:50:07 Done.
86 if (hr == S_OK)
87 {
88 url = std::wstring(result.get(), result_length);
89 }
90 /*
91 * If the call to UrlUnescape fails, we don't alter the string.
92 * This matches the behavior of the previous version of this function.
Felix Dahlke 2014/08/15 15:32:29 How about changing "the previous version" to "the
Eric 2014/09/29 11:50:07 I'm moving this comment to the function header, si
Felix Dahlke 2014/09/30 09:10:28 Yeah, good point. I think the comment remaining he
Eric 2014/09/30 15:22:21 I'll fix the comment again. What's not obvious i
Felix Dahlke 2014/09/30 16:08:16 Fair enough.
93 * Because there's no error handling, however, this masks failures in UrlUne scape.
94 */
95 }
96 catch(...)
97 {
98 // no modification if exception
99 }
100 }
69 101
70 void CPluginClientBase::LogPluginError(DWORD errorCode, int errorId, int errorSu bid, const CString& description, bool isAsync, DWORD dwProcessId, DWORD dwThread Id) 102 void CPluginClientBase::LogPluginError(DWORD errorCode, int errorId, int errorSu bid, const CString& description, bool isAsync, DWORD dwProcessId, DWORD dwThread Id)
71 { 103 {
72 // Prevent circular references 104 // Prevent circular references
73 if (CPluginSettings::HasInstance() && isAsync) 105 if (CPluginSettings::HasInstance() && isAsync)
74 { 106 {
75 DEBUG_ERROR_CODE_EX(errorCode, description, dwProcessId, dwThreadId); 107 DEBUG_ERROR_CODE_EX(errorCode, description, dwProcessId, dwThreadId);
76 108
77 CString pluginError; 109 CString pluginError;
78 pluginError.Format(L"%2.2d%2.2d", errorId, errorSubid); 110 pluginError.Format(L"%2.2d%2.2d", errorId, errorSubid);
(...skipping 39 matching lines...) Expand 10 before | Expand all | Expand 10 after
118 150
119 hasError = true; 151 hasError = true;
120 152
121 s_pluginErrors.erase(it); 153 s_pluginErrors.erase(it);
122 } 154 }
123 } 155 }
124 s_criticalSectionLocal.Unlock(); 156 s_criticalSectionLocal.Unlock();
125 157
126 return hasError; 158 return hasError;
127 } 159 }
OLDNEW
« no previous file with comments | « src/plugin/PluginClientBase.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld