const cleanup round 1(commentary on what I found)
Some of this was included in the patch submission. Some of it I had questions about. spawn.c : warnings may or may not be harmless depending on if esecvp attempts to modify the results. Specs indicate it doesn't, but the function typing doesn't indicate so(the difference is 'const char * const argv[]' and 'char *const argv[]') libs/wine/string.c: strstrW - returns the location where sub was found in str. I believe returning a non-const value is what is expected. Should this be changed to a non-const parameter, or just leave the warning as is? tools/winedump/pdb.c: pdb_exit [line 149,153,155] - freeing of const variable. Is this ok? I could imagine it *could* cause problems if what we are freeing isn't in readable/writeable memory. pdb_dump_symbols [line 293,403,409] - same as pdb_exit. pdb_dump_types [line 462] - ditto pdb_jg_dump line[514] - ditto pdb_ds_dump line[643] - ditto tools/winedump/search.c: get_type [line 305, 329] - freeing of const variable again. [line 314, 335] - both cases, set return_text to type_str. Either parsed_symbol's return_text needs to be constified, proto should be unconstified, or leave it in place as a necessary cast. Any comments on the above spots? what of these are necessary casts? what of these can/should be corrected, and what needs to be done to correct them? **patches submitted to wine-patches** the following were straight forward patches that shouldn't modify the flow of the program beyond fixing the warnings. libs/wine/string.c: strtolW and strtoulW - constify the endptr value. This is just an output value to indicate where in the array we stoped processing. Change causes this to be the same type as the first value. Reduces number of cast-qual warnings by 6. tools/winedump/lnk.c: dump_pidl [line 176] - switched to const. Value is never modified in the function. Removes a cast-qual warning. tools/winedump/symbol.c: symbol_clean_string - this function is modifying the string. This should most definitely *not* be a const parameter. It's not even logically const(as a comment tried to claim) as the the intent of the function is to make the type string Wine-friendly. That requires modifications to that string, and to the variable. Removes a cast-qual warning. Comments? questions? Current number of cast-qual warnings fixed: 8. Number deemed necessary: 0(you guys need to help me decide what of these need to remain) Number looked at: 24. Estimated number remaining: 521.
Re: const cleanup round 1
after some extra testing I found that it's not that my modification would forbid that, but it would complain about the second parameter not being const(which I thought that the compiler would be fine with auto promoting the WCHAR ** to const WCHAR **). Fair enoug then. Strike those modifications. Should I resubmit without those? or should I put that off until I get a second round ready? On 3/13/07, Eric Pouech [EMAIL PROTECTED] wrote: Mike Schaadt a écrit : string.c: strtolW and strtoulW - constify the endptr value. This is just an output value to indicate where in the array we stoped processing. Change causes this to be the same type as the first value. Reduces number of cast-qual warnings by 6. lnk.c: dump_pidl [line 176] - switched to const. Value is never modified in the function. Removes a cast-qual warning. symbol.c: symbol_clean_string - this function is modifying the string. This should most definitly *not* be a const parameter. It's not even logically const as the the intent of the function is to make the type string Wine-friendly. That requires modifications to that string. diff --git a/include/wine/unicode.h b/include/wine/unicode.h index 83a7d52..4e2caa5 100644 --- a/include/wine/unicode.h +++ b/include/wine/unicode.h @@ -96,8 +96,8 @@ extern int strcmpiW( const WCHAR *str1, extern int strncmpiW( const WCHAR *str1, const WCHAR *str2, int n ); extern int memicmpW( const WCHAR *str1, const WCHAR *str2, int n ); extern WCHAR *strstrW( const WCHAR *str, const WCHAR *sub ); -extern long int strtolW( const WCHAR *nptr, WCHAR **endptr, int base ); -extern unsigned long int strtoulW( const WCHAR *nptr, WCHAR **endptr, int base ); +extern long int strtolW( const WCHAR *nptr, const WCHAR **endptr, int base ); +extern unsigned long int strtoulW( const WCHAR *nptr, const WCHAR **endptr, int base ); this is wrong as you want to be able to write code as: WCHAR* str; WCHAR* p; int val = strtolW(str, p, 10); *p = '\0'; which your modification forbids (ANSI strtol doesn't have the const modifier for endptr) the rest of the patch looks ok -- Eric Pouech The problem with designing something completely foolproof is to underestimate the ingenuity of a complete idiot. (Douglas Adams)
Re: const cleanup round 1
Mike Schaadt a écrit : after some extra testing I found that it's not that my modification would forbid that, but it would complain about the second parameter not being const(which I thought that the compiler would be fine with auto promoting the WCHAR ** to const WCHAR **). Fair enoug then. Strike those modifications. Should I resubmit without those? or should I put that off until I get a second round ready? you'd better remove that part from the part, and resubmit the good part A+ -- Eric Pouech The problem with designing something completely foolproof is to underestimate the ingenuity of a complete idiot. (Douglas Adams)
