WININET: make WCHAR strings const where possible
"Dmitry Timoshkov" <dmitry(a)baikal.ru> wrote:
It would be much better if you could mark all string constants as such using 'const' key word in all your patches (including already committed, submitted and future ones).
Perhaps we should make a janitorial task to CONST-ify strings where possible? Mike ChangeLog: * make WCHAR strings const where possible Index: dlls/wininet/http.c =================================================================== RCS file: /home/wine/wine/dlls/wininet/http.c,v retrieving revision 1.56 diff -u -r1.56 http.c --- dlls/wininet/http.c 31 Mar 2004 20:09:54 -0000 1.56 +++ dlls/wininet/http.c 12 Apr 2004 14:12:48 -0000 @@ -470,8 +470,8 @@ { UINT len; LPWSTR in, out; - WCHAR szBasic[] = {'B','a','s','i','c',' ',0}; - WCHAR szColon[] = {':',0}; + const WCHAR szBasic[] = {'B','a','s','i','c',' ',0}; + const WCHAR szColon[] = {':',0}; len = lstrlenW( username ) + 1 + lstrlenW ( password ) + 1; in = HeapAlloc( GetProcessHeap(), 0, len*sizeof(WCHAR) ); @@ -504,7 +504,7 @@ { HTTPHEADERW hdr; INT index; - WCHAR szProxyAuthorization[] = { + const WCHAR szProxyAuthorization[] = { 'P','r','o','x','y','-','A','u','t','h','o','r','i','z','a','t','i','o','n',0 }; hdr.lpszValue = HTTP_EncodeBasicAuth( username, password ); @@ -538,10 +538,9 @@ WCHAR proxy[MAXHOSTNAME + 15]; /* 15 == "http://" + sizeof(port#) + ":/\0" */ WCHAR* url, szNul[] = { 0 }; URL_COMPONENTSW UrlComponents; - WCHAR szHttp[] = { 'h','t','t','p',':','/','/',0 }, szSlash[] = { '/',0 } ; - /*WCHAR szColon[] = { ':',0 }; */ - WCHAR szFormat1[] = { 'h','t','t','p',':','/','/','%','s',':','%','d',0 }; - WCHAR szFormat2[] = { 'h','t','t','p',':','/','/','%','s',':','%','d',0 }; + const WCHAR szHttp[] = { 'h','t','t','p',':','/','/',0 }, szSlash[] = { '/',0 } ; + const WCHAR szFormat1[] = { 'h','t','t','p',':','/','/','%','s',':','%','d',0 }; + const WCHAR szFormat2[] = { 'h','t','t','p',':','/','/','%','s',':','%','d',0 }; int len; memset( &UrlComponents, 0, sizeof UrlComponents ); @@ -607,7 +606,7 @@ LPWSTR lpszUrl = NULL; DWORD nCookieSize; HINTERNET handle; - WCHAR szUrlForm[] = {'h','t','t','p',':','/','/','%','s',0}; + const WCHAR szUrlForm[] = {'h','t','t','p',':','/','/','%','s',0}; DWORD len; TRACE("--> \n"); @@ -670,7 +669,7 @@ if (NULL == lpszVerb) { - WCHAR szGet[] = {'G','E','T',0}; + const WCHAR szGet[] = {'G','E','T',0}; lpwhr->lpszVerb = WININET_strdupW(szGet); } else if (strlenW(lpszVerb)) @@ -698,7 +697,7 @@ if (hIC->lpszAgent) { WCHAR *agent_header; - WCHAR user_agent[] = {'U','s','e','r','-','A','g','e','n','t',':',' ','%','s','\r','\n',0 }; + const WCHAR user_agent[] = {'U','s','e','r','-','A','g','e','n','t',':',' ','%','s','\r','\n',0 }; len = strlenW(hIC->lpszAgent) + strlenW(user_agent); agent_header = HeapAlloc( GetProcessHeap(), 0, len*sizeof(WCHAR) ); @@ -716,8 +715,8 @@ if (InternetGetCookieW(lpszUrl, NULL, NULL, &nCookieSize)) { int cnt = 0; - WCHAR szCookie[] = {'C','o','o','k','i','e',':',' ',0}; - WCHAR szcrlf[] = {'\r','\n',0}; + const WCHAR szCookie[] = {'C','o','o','k','i','e',':',' ',0}; + const WCHAR szcrlf[] = {'\r','\n',0}; lpszCookies = HeapAlloc(GetProcessHeap(), 0, (nCookieSize + 1 + 8)*sizeof(WCHAR)); @@ -789,9 +788,9 @@ LPHTTPHEADERW lphttpHdr = NULL; BOOL bSuccess = FALSE; LPWININETHTTPREQW lpwhr; - WCHAR szFmt[] = { '%','s',':',' ','%','s','%','s',0 }; - WCHAR szcrlf[] = { '\r','\n',0 }; - WCHAR sznul[] = { 0 }; + const WCHAR szFmt[] = { '%','s',':',' ','%','s','%','s',0 }; + const WCHAR szcrlf[] = { '\r','\n',0 }; + const WCHAR sznul[] = { 0 }; if (TRACE_ON(wininet)) { #define FE(x) { x, #x } @@ -1419,12 +1418,12 @@ do { - WCHAR szSlash[] = { '/',0 }; - WCHAR szSpace[] = { ' ',0 }; - WCHAR szHttp[] = { 'h','t','t','p',':','/','/', 0 }; - WCHAR szcrlf[] = {'\r','\n', 0}; - WCHAR sztwocrlf[] = {'\r','\n','\r','\n', 0}; - WCHAR szSetCookie[] = {'S','e','t','-','C','o','o','k','i','e',0 }; + const WCHAR szSlash[] = { '/',0 }; + const WCHAR szSpace[] = { ' ',0 }; + const WCHAR szHttp[] = { 'h','t','t','p',':','/','/', 0 }; + const WCHAR szcrlf[] = {'\r','\n', 0}; + const WCHAR sztwocrlf[] = {'\r','\n','\r','\n', 0}; + const WCHAR szSetCookie[] = {'S','e','t','-','C','o','o','k','i','e',0 }; TRACE("Going to url %s %s\n", debugstr_w(lpwhr->lpszHostName), debugstr_w(lpwhr->lpszPath)); loop_next = FALSE; @@ -1540,7 +1539,7 @@ { if (lpwhr->StdHeaders[i].wFlags & HDR_ISREQUEST) { - WCHAR szFmt[] = { '\r','\n','%','s',':',' ','%','s', 0}; + const WCHAR szFmt[] = { '\r','\n','%','s',':',' ','%','s', 0}; cnt += sprintfW(requestString + cnt, szFmt, lpwhr->StdHeaders[i].lpszField, lpwhr->StdHeaders[i].lpszValue); TRACE("Adding header %s (%s)\n", @@ -1554,7 +1553,7 @@ { if (lpwhr->pCustHeaders[i].wFlags & HDR_ISREQUEST) { - WCHAR szFmt[] = { '\r','\n','%','s',':',' ','%','s', 0}; + const WCHAR szFmt[] = { '\r','\n','%','s',':',' ','%','s', 0}; cnt += sprintfW(requestString + cnt, szFmt, lpwhr->pCustHeaders[i].lpszField, lpwhr->pCustHeaders[i].lpszValue); TRACE("Adding custom header %s (%s)\n", @@ -1565,7 +1564,7 @@ if (lpwhr->lpszHostName) { - WCHAR szFmt[] = { '%','s','%','s',0 }; + const WCHAR szFmt[] = { '%','s','%','s',0 }; cnt += sprintfW(requestString + cnt, szFmt, HTTPHOSTHEADER, lpwhr->lpszHostName); } @@ -1658,7 +1657,7 @@ { LPHTTPHEADERW setCookieHeader; int nPosStart = 0, nPosEnd = 0, len; - WCHAR szFmt[] = { 'h','t','t','p',':','/','/','%','s','/',0}; + const WCHAR szFmt[] = { 'h','t','t','p',':','/','/','%','s','/',0}; setCookieHeader = &lpwhr->pCustHeaders[CustHeaderIndex]; @@ -1678,7 +1677,7 @@ /* fixme: not case sensitive, strcasestr is gnu only */ int nDomainPosEnd = 0; int nDomainPosStart = 0, nDomainLength = 0; - WCHAR szDomain[] = {'d','o','m','a','i','n','=',0}; + const WCHAR szDomain[] = {'d','o','m','a','i','n','=',0}; LPWSTR lpszDomain = strstrW(&setCookieHeader->lpszValue[nPosEnd], szDomain); if (lpszDomain) { /* they have specified their own domain, lets use it */ @@ -1952,8 +1951,8 @@ BOOL bSuccess = FALSE; INT rc = 0; WCHAR value[MAX_FIELD_VALUE_LEN], field[MAX_FIELD_LEN]; - WCHAR szStatus[] = {'S','t','a','t','u','s',0}; - WCHAR szHttp[] = { 'H','T','T','P',0 }; + const WCHAR szStatus[] = {'S','t','a','t','u','s',0}; + const WCHAR szHttp[] = { 'H','T','T','P',0 }; char bufferA[MAX_REPLY_LEN]; TRACE("-->\n"); @@ -2087,32 +2086,32 @@ INT HTTP_GetStdHeaderIndex(LPCWSTR lpszField) { INT index = -1; - WCHAR szContentLength[] = { + const WCHAR szContentLength[] = { 'C','o','n','t','e','n','t','-','L','e','n','g','t','h',0}; - WCHAR szStatus[] = {'S','t','a','t','u','s',0}; - WCHAR szContentType[] = { + const WCHAR szStatus[] = {'S','t','a','t','u','s',0}; + const WCHAR szContentType[] = { 'C','o','n','t','e','n','t','-','T','y','p','e',0}; - WCHAR szLastModified[] = { + const WCHAR szLastModified[] = { 'L','a','s','t','-','M','o','d','i','f','i','e','d',0}; - WCHAR szLocation[] = {'L','o','c','a','t','i','o','n',0}; - WCHAR szAccept[] = {'A','c','c','e','p','t',0}; - WCHAR szReferer[] = { 'R','e','f','e','r','e','r',0}; - WCHAR szContentTrans[] = { 'C','o','n','t','e','n','t','-', + const WCHAR szLocation[] = {'L','o','c','a','t','i','o','n',0}; + const WCHAR szAccept[] = {'A','c','c','e','p','t',0}; + const WCHAR szReferer[] = { 'R','e','f','e','r','e','r',0}; + const WCHAR szContentTrans[] = { 'C','o','n','t','e','n','t','-', 'T','r','a','n','s','f','e','r','-','E','n','c','o','d','i','n','g',0}; - WCHAR szDate[] = { 'D','a','t','e',0}; - WCHAR szServer[] = { 'S','e','r','v','e','r',0}; - WCHAR szConnection[] = { 'C','o','n','n','e','c','t','i','o','n',0}; - WCHAR szETag[] = { 'E','T','a','g',0}; - WCHAR szAcceptRanges[] = { + const WCHAR szDate[] = { 'D','a','t','e',0}; + const WCHAR szServer[] = { 'S','e','r','v','e','r',0}; + const WCHAR szConnection[] = { 'C','o','n','n','e','c','t','i','o','n',0}; + const WCHAR szETag[] = { 'E','T','a','g',0}; + const WCHAR szAcceptRanges[] = { 'A','c','c','e','p','t','-','R','a','n','g','e','s',0 }; - WCHAR szExpires[] = { 'E','x','p','i','r','e','s',0 }; - WCHAR szMimeVersion[] = { + const WCHAR szExpires[] = { 'E','x','p','i','r','e','s',0 }; + const WCHAR szMimeVersion[] = { 'M','i','m','e','-','V','e','r','s','i','o','n', 0}; - WCHAR szPragma[] = { 'P','r','a','g','m','a', 0}; - WCHAR szCacheControl[] = { + const WCHAR szPragma[] = { 'P','r','a','g','m','a', 0}; + const WCHAR szCacheControl[] = { 'C','a','c','h','e','-','C','o','n','t','r','o','l',0}; - WCHAR szUserAgent[] = { 'U','s','e','r','-','A','g','e','n','t',0}; - WCHAR szProxyAuth[] = { + const WCHAR szUserAgent[] = { 'U','s','e','r','-','A','g','e','n','t',0}; + const WCHAR szProxyAuth[] = { 'P','r','o','x','y','-', 'A','u','t','h','e','n','t','i','c','a','t','e', 0};
On Tue, 13 Apr 2004, Mike McCormack wrote: [...]
Perhaps we should make a janitorial task to CONST-ify strings where possible?
In some places the Unicode strings that are declared *inside functions* are declared both static and const. Is the static necessary in that context? The following command can help in this janitorial task: grep -r WCHAR . | egrep "\\[\\] *= *{ *" | grep -v const -- Francois Gouget fgouget(a)free.fr http://fgouget.free.fr/ We are Pentium of Borg. You will be approximated. Division is futile.
Francois Gouget wrote:
On Tue, 13 Apr 2004, Mike McCormack wrote: [...]
Perhaps we should make a janitorial task to CONST-ify strings where possible?
In some places the Unicode strings that are declared *inside functions* are declared both static and const. Is the static necessary in that context?
The static is even more necessary in that context. Outside of functions, all static does is limit the scope. Inside functions, static prevents the var from taking stack space. Shachar -- Shachar Shemesh Lingnu Open Source Consulting http://www.lingnu.com/
On Tue, Apr 13, 2004 at 11:40:00PM +0300, Shachar Shemesh wrote:
Francois Gouget wrote:
On Tue, 13 Apr 2004, Mike McCormack wrote:
Perhaps we should make a janitorial task to CONST-ify strings where possible? In some places the Unicode strings that are declared *inside functions* are declared both static and const. Is the static necessary in that context? The static is even more necessary in that context. Outside of functions, all static does is limit the scope. Inside functions, static prevents the var from taking stack space. Afaik, if it's const it should be somewhere in the .bss and therefor it shouldn't be allocated on the stack on function entry.
bye michael -- Michael Stefaniuc Tel.: +49-711-96437-199 System Administration Fax.: +49-711-96437-111 Red Hat GmbH Email: mstefani(a)redhat.com Hauptstaetterstr. 58 http://www.redhat.de/ D-70178 Stuttgart
Michael Stefaniuc wrote:
Afaik, if it's const it should be somewhere in the .bss and therefor it shouldn't be allocated on the stack on function entry.
bye michael
That MAY be true for C++. It is defenitely not true for C. In C, const just means it's the same ol' var, only it can't be changed. const is a pretty late addition to C, and actually got there from C++. -- Shachar Shemesh Lingnu Open Source Consulting http://www.lingnu.com/
On Tue, 13 Apr 2004, Shachar Shemesh wrote: [...]
are declared both static and const. Is the static necessary in that context?
The static is even more necessary in that context. Outside of functions, all static does is limit the scope. Inside functions, static prevents the var from taking stack space.
I wrote a teeny test app to check this out and you're right. The static is crucial. If it's not specified the string is copied to the stack so that the const essentially has no effect whatsoever (no compiler warning and no runtime error). With 'static const' we still don't get a compiler warning (tested with gcc version 3.3.3 (Debian 20040321)) but we get a runtime crash, i.e. the string is really const this time. #include <stdio.h> void foo() { /*static*/ /*const*/ char buf[]={'a',':','b',':','c',':','d',0}; char* c; c=strchr(buf,':'); if (c) *c=' '; printf("buf=%s\n",buf); } int main() { foo(); foo(); foo(); return 0; } -- Francois Gouget fgouget(a)free.fr http://fgouget.free.fr/ Linux: the choice of a GNU generation
Francois Gouget wrote:
I wrote a teeny test app to check this out and you're right. The static
is crucial. If it's not specified the string is copied to the stack so that the const essentially has no effect whatsoever (no compiler warning and no runtime error). With 'static const' we still don't get a compiler warning (tested with gcc version 3.3.3 (Debian 20040321)) but we get a runtime crash, i.e. the string is really const this time.
Now try the same prog, only compile it with g++ instead of gcc. Hmm - doesn't seem to have any important effect :-\ -- Shachar Shemesh Lingnu Open Source Consulting http://www.lingnu.com/
Francois Gouget wrote:
I wrote a teeny test app to check this out and you're right. The static
is crucial. If it's not specified the string is copied to the stack so that the const essentially has no effect whatsoever (no compiler warning and no runtime error). With 'static const' we still don't get a compiler warning (tested with gcc version 3.3.3 (Debian 20040321)) but we get a runtime crash, i.e. the string is really const this time.
Of course there is no warning. You are casting away the "const" by using strchr. The lack of overloading means that it must be defined as accepting const pointer, but returning non-const pointer. In effect, it removes the pointer's constantness. Try implementing it yourself, and you will get warnings in both languages. Shachar -- Shachar Shemesh Lingnu Open Source Consulting http://www.lingnu.com/
participants (4)
-
Francois Gouget -
Michael Stefaniuc -
Mike McCormack -
Shachar Shemesh