Summary: | constness of argument names in XInternAtoms() is incorrect. | ||
---|---|---|---|
Product: | xorg | Reporter: | Robert White <rwhite> |
Component: | Lib/Xlib | Assignee: | Alan Coopersmith <alan.coopersmith> |
Status: | RESOLVED FIXED | QA Contact: | Xorg Project Team <xorg-team> |
Severity: | normal | ||
Priority: | medium | CC: | remi |
Version: | git | ||
Hardware: | All | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: |
Description
Robert White
2010-12-04 04:29:58 UTC
Except that Xlib is C, not C++, and C doesn't do that: http://stackoverflow.com/questions/78125/why-cant-i-convert-char-to-a-const-char-const-in-c If you have a working patch that fixes warnings with gcc, not g++, I'd love to see it. (In reply to comment #1) > Except that Xlib is C, not C++, and C doesn't do that: > http://stackoverflow.com/questions/78125/why-cant-i-convert-char-to-a-const-char-const-in-c > > If you have a working patch that fixes warnings with gcc, not g++, > I'd love to see it. Good point... Using: Func(const char *names[]); instead of Func(const char **names); does everything needed for both languages and silences the warnings and errors in each. Least to my testing. It should be a drop-in semantic replacement in both cases. The use of an array instead of a pointer prevents modification of the resulting list-o-pointers and so prevents the possible error. This, in turn, propigates the immutable nature of the list safely and so is the same as the second const in char const * const *. (In reply to comment #2) > The use of an array instead of a pointer prevents modification of the > resulting list-o-pointers and so prevents the possible error. I was wrong about the why. using the array style doesn't prevent the modification of the pointers. It unambiguously propagates the "point to const char" type through the function call. This then prevents the accidental misuse of the constant pointer as a non-constant pointer. Either way, the use of "const type **var" can almost always be more clearly expressed as "const type *var[]" as long as the called function doesn't get all upset about needing a local (or subscripting) to iterate over var. Actually, you may even want void Func(char const * const names[]); I _think_ that will constantize both the character strings and the vector of pointers. Using the test case of the libXt/src/Selection.c code currently throwing warnings over: static char* names[] = { ... }; XInternAtoms(dpy, names, 4, FALSE, atoms); This declaration in Xlib.h: extern Status XInternAtoms( Display* /* dpy */, _Xconst char * names[], int /* count */, Bool /* onlyIfExists */, Atom* /* atoms_return */ ); Still results in gcc 3.4.3 reporting: Selection.c: In function `GetPropList': Selection.c:191: warning: passing arg 2 of `XInternAtoms' from incompatible pointer type Sun Studio 12.1 reports: "Selection.c", line 191: warning: argument #2 is incompatible with prototype: prototype: pointer to pointer to const char : "/net/also.us.oracle.com/export/alanc/X.Org/sx86/install/usr/X11R7/include/X11/Xlib.h", line 1549 argument : pointer to pointer to char With the Xlib.h declaration changed to: char const * const names[], gcc 3.4.3 & 4.3.2 report: Selection.c: In function `GetPropList': Selection.c:191: warning: passing arg 2 of `XInternAtoms' from incompatible pointer type Sun Studio 12.1 reports: "Selection.c", line 191: warning: argument #2 is incompatible with prototype: prototype: pointer to const pointer to const char : "/net/also.us.oracle.com/export/alanc/X.Org/sx86/install/usr/X11R7/include/X11/Xlib.h", line 1549 argument : pointer to pointer to char Do you have an example of either of these declarations fixing warnings from code that is currently issuing them? (Xt may not be the best test case, but its one we have) Yep. Turns out I am an idiot, or at least I was last night. The code I was using to double-check my assertions contained one of the stupidest programming mistakes I've made in years. Now I'm having flashbacks to an embedded platform compiler where I ran into this some years back and I ended up just dropping "const" from most virtually all of my vector of character handling. I came to the conclusion back then that somewhere back in prehistory, when someone decided to make 'char * boo = "ya";' [e.g. silently permit/promote assigning a constant string to a non-constant character pointer] a legal construct, they did so because they realized that the "constness of pointer-to-pointer" problem was largely unsolvable in C. What is odd is that "const char **name;" does have the "intermediate pointer movement problem" but "char const * const name[];" shouldn't as you cannot move name and you should not be able to alter the elements of name to redirect them. I'll bet that this is why parameter "names" wasn't constant in XInternAtoms() to begin with... So like never-mind on my corrections. But the addition of the new const is killing (build-breaking) a lot of applications out here in the real world (kdelibs, xulrunner, etc) so maybe it ought not be checked in just now...? Just piping in to say we've been bit by this as well. https://bugs.gentoo.org/347603 @Alan, I'm not sure if this situation is solvable at all, C++ being much stricter. Wouldn't it be better to revert back to the old un-const signatures, at least until someone figures it out? Cheers Proposed solution: http://lists.x.org/archives/xorg-devel/2010-December/017084.html Example usage: http://lists.x.org/archives/xorg-devel/2010-December/017085.html As Daniel pointed out on xorg-devel, the proposed solution requires all libraries & the application being compiled to agree on the constness. For lack of any good ideas at resolving, I've accepted defeat and reverted, and we've all learned a lesson about constness in C being harder than it appears: http://cgit.freedesktop.org/xorg/lib/libX11/commit/?id=d2714d65e85b44abedf5f82e1a31506dba397ef2 |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.