Re: Wide-string Functions: Double Casting
David Laight wrote:
On Tue, Aug 08, 2006 at 09:27:23PM +0200, Eric Pouech wrote:
what I don't like is that in order to plug a hole (casting from const
foo* to foo*), we create a bigger hole by allowing to cast from const
foo* to bar* (and the compiler will not give any warning)
if we want to go into this, then I'd rather have _deconst explicitly use
the type:
#define __deconst(v,X)({const X _v = (v); ((X)(int)_v);})
and in the previous example use __deconst(str, char*);
That isn't valid C, you could do:
((const X)(v) - (v), (X)(int)(v))
but that is likely to give a 'computed value not used' warning instead.
What you really don't want to do is allow casts from 'int' to 'foo *'.
After all casting from 'foo *' to 'bar *' is easy.
David
this is supported by gcc...
of course we'd need to define differently the macro for gcc and other
compilers
but casting from foo* to bar* is (in most of the cases) a sign of bad design
A+
Re: Wide-string Functions: Double Casting
Once qualifications are give, how hard it is to take them away, again!
Of course, if one were realising these small inline functions literally as
inline code, one would not add a const qualification for a few lines, only
to take it away again (with great difficulty).
Thus, one would not write:
static WCHAR s[] = {'h','e','l','l','o',0};
LPCWSTR ps1;/* [1] */
LPWSTR ps2;
const WCHAR ch = 'h';
ps1 = s;
ps2 = NULL;
do
{
if (*ps1 == ch)
{
ps2 = (LPWSTR)(uintptr_t)ps1; /* [2] */
break;
}
} while (*ps1++);
if (ps2 != NULL)
*ps2 = 'y';
... when [1] need only be
LPWSTR;
and [2} need only be
ps2 = ps1;
So there might be some argument for specially declaring these few functions
- suitably commented - as taking unqualified string parameters. (The
problem with constifying both input and output pointers is that one might
wish to use the returned pointer to modify the input string.)
However, now that the ANSI/ISO people have given use intptr_t and uintptr_t
(in C99), I don't see what is wrong with, in effect, replacing
ret = (WCHAR *)str;
with
ret = (WCHAR *)(uintptr_t)str;
To avoid touching Wine's configuration files (to add [u]intptr_t for non-C99
systems) the intermediate cast might be have been (unsigned long). Maybe,
(size_t) would have worked, and been more indicative of use(?) unsigned
long should have met the needs of the platforms for which we cater. The
spoiler, apparently, is that it won't work with Win64 type definitions.
Though, I don't know the details.
Anyway, I shall look at fixing the straightforward cast-qual violations,
first, and revisit the controversial issues until the end.
-- Andy.
Re: Wide-string Functions: Double Casting
On Mon, Aug 07, 2006 at 09:54:20PM +0100, Andrew Talbot wrote:
Although I accept that my opinion may not be universally shared :-), I
believe that it is better to turn -Wcast-qual on permanently and
double-cast the appropriate return values of the relevant wide-string
functions (strchrW(), strrchrW(), strpbrkW(), memchrW() and memrchrW())
than to leave it normally turned off: that way, the developers can avoid
violations as they go, and give the janitors a break ;-). These functions
are ubiquitous and widely understood, and they account for the vast
majority of cast-qual warnings.
Because, as I understand it, we are aiming for C89 compatibility, and to try
to maximise portability, I am proposing to use size_t, rather than
uintptr_t (which is a C99 type): it seems like the next-best thing. So I
would like to submit a patch that, for example, changes strchrW() to:
extern inline WCHAR *strrchrW( const WCHAR *str, WCHAR ch )
{
WCHAR *ret = NULL;
do { if (*str == ch) ret = (WCHAR *)(size_t)str; } while (*str++);
return ret;
}
What do people think? Should I just send in the patch and see what criticism
it engenders? Does anyone have a system on which this would fail or
generate other warnings in place of the cast-qual one?
Needless to say, I hope to be tackling the cast-qual janitorial task, soon;
hence, my sudden interest in this.
I asked our gcc gurus.
- If you want to cast, do not use size_t but uintptr_t.
Or:
- Do not inline the function, and compile its file without -Wcast-qual.
- Fix the prototype to read extern inline const WCHAR *strrchrW( const WCHAR
*str, WCHAR ch )
Ciao, Marcus
Re: Wide-string Functions: Double Casting
Marcus Meissner wrote:
I asked our gcc gurus.
- If you want to cast, do not use size_t but uintptr_t.
Or:
- Do not inline the function, and compile its file without -Wcast-qual.
- Fix the prototype to read extern inline const WCHAR *strrchrW( const
WCHAR *str, WCHAR ch )
Ciao, Marcus
intptr_t and uintptr_t were, of course, introduced in C99 specifically to be
integer types to which any valid pointer to an object may be converted.
This is the ultimate way to go, I believe, but I would need help or advice
to patch the appropriate make file to cater for users with non-C99 systems.
Incidentally, regarding the other suggested solutions: IMHO, I don't think
it a good idea to remove the inlining, thus adding the overhead of a
function call to these functions. And if one constifies the return value,
then it becomes no longer possible to write code like:
WCHAR arrW[] = {'b','i','t',0}, *pwc;
if ((pwc = strrchrW(arrW, 'i')) != NULL)
*pwc = 'a';
Thanks,
-- Andy.
Re: Wide-string Functions: Double Casting
Andrew Talbot wrote:
David Laight wrote:
On Mon, Aug 07, 2006 at 09:54:20PM +0100, Andrew Talbot wrote:
would like to submit a patch that, for example, changes strchrW() to:
extern inline WCHAR *strrchrW( const WCHAR *str, WCHAR ch )
{
WCHAR *ret = NULL;
do { if (*str == ch) ret = (WCHAR *)(size_t)str; } while (*str++);
return ret;
}
why not just have:
extern inline void *__deconst(const void *v)
{
return (char *)0 + ((const char *)v - (const char *)0));
}
Then the code above could be:
extern inline WCHAR *strrchrW( const WCHAR *str, WCHAR ch )
{
do { if (*str == ch) return __deconst(str); } while (*str++);
return 0;
}
David
That's interesting. IMHO, it's a bit more complex and obscure, but seems to
ensure portability. I shall be interested to know what others think.
what I don't like is that in order to plug a hole (casting from const
foo* to foo*), we create a bigger hole by allowing to cast from const
foo* to bar* (and the compiler will not give any warning)
if we want to go into this, then I'd rather have _deconst explicitly use
the type:
#define __deconst(v,X)({const X _v = (v); ((X)(int)_v);})
and in the previous example use __deconst(str, char*);
one could think of using gcc's typeof operator to avoid passing the type
to deconst, but this doesn't seem possible (you have to express foo* in
terms of const foo* which isn't possible AFAICT)
A+
Re: Wide-string Functions: Double Casting
On Tue, Aug 08, 2006 at 09:27:23PM +0200, Eric Pouech wrote:
what I don't like is that in order to plug a hole (casting from const
foo* to foo*), we create a bigger hole by allowing to cast from const
foo* to bar* (and the compiler will not give any warning)
if we want to go into this, then I'd rather have _deconst explicitly use
the type:
#define __deconst(v,X)({const X _v = (v); ((X)(int)_v);})
and in the previous example use __deconst(str, char*);
That isn't valid C, you could do:
((const X)(v) - (v), (X)(int)(v))
but that is likely to give a 'computed value not used' warning instead.
What you really don't want to do is allow casts from 'int' to 'foo *'.
After all casting from 'foo *' to 'bar *' is easy.
David
--
David Laight: [EMAIL PROTECTED]
Wide-string Functions: Double Casting
Although I accept that my opinion may not be universally shared :-), I
believe that it is better to turn -Wcast-qual on permanently and
double-cast the appropriate return values of the relevant wide-string
functions (strchrW(), strrchrW(), strpbrkW(), memchrW() and memrchrW())
than to leave it normally turned off: that way, the developers can avoid
violations as they go, and give the janitors a break ;-). These functions
are ubiquitous and widely understood, and they account for the vast
majority of cast-qual warnings.
Because, as I understand it, we are aiming for C89 compatibility, and to try
to maximise portability, I am proposing to use size_t, rather than
uintptr_t (which is a C99 type): it seems like the next-best thing. So I
would like to submit a patch that, for example, changes strchrW() to:
extern inline WCHAR *strrchrW( const WCHAR *str, WCHAR ch )
{
WCHAR *ret = NULL;
do { if (*str == ch) ret = (WCHAR *)(size_t)str; } while (*str++);
return ret;
}
What do people think? Should I just send in the patch and see what criticism
it engenders? Does anyone have a system on which this would fail or
generate other warnings in place of the cast-qual one?
Needless to say, I hope to be tackling the cast-qual janitorial task, soon;
hence, my sudden interest in this.
Thanks,
-- Andy.
Re: Wide-string Functions: Double Casting
Andrew Talbot wrote: [...] So I would like to submit a patch that, for example, changes strchrW() to: extern inline WCHAR *strrchrW( const WCHAR *str, WCHAR ch ) [...] Yes, that was a typo: it should say changes strrchrW() -- Andy.
Re: Wide-string Functions: Double Casting
On Mon, Aug 07, 2006 at 09:54:20PM +0100, Andrew Talbot wrote:
would like to submit a patch that, for example, changes strchrW() to:
extern inline WCHAR *strrchrW( const WCHAR *str, WCHAR ch )
{
WCHAR *ret = NULL;
do { if (*str == ch) ret = (WCHAR *)(size_t)str; } while (*str++);
return ret;
}
why not just have:
extern inline void *__deconst(const void *v)
{
return (char *)0 + ((const char *)v - (const char *)0));
}
Then the code above could be:
extern inline WCHAR *strrchrW( const WCHAR *str, WCHAR ch )
{
do { if (*str == ch) return __deconst(str); } while (*str++);
return 0;
}
David
--
David Laight: [EMAIL PROTECTED]
Re: Wide-string Functions: Double Casting
David Laight wrote:
On Mon, Aug 07, 2006 at 09:54:20PM +0100, Andrew Talbot wrote:
would like to submit a patch that, for example, changes strchrW() to:
extern inline WCHAR *strrchrW( const WCHAR *str, WCHAR ch )
{
WCHAR *ret = NULL;
do { if (*str == ch) ret = (WCHAR *)(size_t)str; } while (*str++);
return ret;
}
why not just have:
extern inline void *__deconst(const void *v)
{
return (char *)0 + ((const char *)v - (const char *)0));
}
Then the code above could be:
extern inline WCHAR *strrchrW( const WCHAR *str, WCHAR ch )
{
do { if (*str == ch) return __deconst(str); } while (*str++);
return 0;
}
David
That's interesting. IMHO, it's a bit more complex and obscure, but seems to
ensure portability. I shall be interested to know what others think.
Thanks,
-- Andy.
