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
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 eric.pouech@wanadoo.fr 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)
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+