Bug in wimp.h

Jonathan Coxhead jonathan at doves.demon.co.uk
Tue Aug 22 01:50:55 BST 2000


On 9 Aug 00, at 21:14, Chris Rutter wrote:

 | On Wed, 09 Aug 2000 08:42:35 Tony van der Hoff wrote:
 | 
 | > This does indeed look wrong. However, is it actually a problem? 
 | 
 | Well, potentially; take the example code:
 | 
 |     char nm[12]; 
 |     nm[0] = '*';
 |     nm[1] = '\0';
 |     wimp_load_template(...); /* Fetch the name of the next template */
 |     for (i = 1; i < 12; i++)
 |       /* ... do something with nm[i] ... */
 | 
 | The compiler's perfectly at liberty to feed you back nm[0] == '*' and
 | nm[1] == '\0' after that call, when clearly those memory locations will 
have
 | been updated.

   Actually, it can't. The inside of wimp_load_template() (if written in C) 
would be completely within its rights to contain code like this:

      wimp_load_template (..., char const *name, ...)
      {
         char *name_var = (char *) name; /*cast away const*/
         name [0] = ...;
      }

and indeed, that's pretty much what it does.

   But there *is* a problem, and here it is:

      char const nm [12] = "*";
      wimp_load_template (..., nm, ...);

Here, |nm| is actually const. It could be allocated in read-only memory, 
for example. So casting away const inside is illegal. (In the first 
example, |nm| was a variable.) Of course, the inside of the function has no 
way of knowing whether its argument is "really" const or not, which is why 
casting away const is a bad thing to do. Nevertheless, it is legal code.

   So, Tony's right, and we should fix the problem.

 | `const' is very very important: it should never be left out when it can 
be
 | put it, but more than that, it should never be put it when it's not 
valid to
 | do so.  Very very confusing and bad things can happen otherwise.
 | 
 | Having said all that, I can't persuade gcc 2.95/i386 or Norcroft C 5 to
 | actually `do the wrong thing'; mainly because they always seem to use a1-
-a4
 | for the sorts of values which might get corrupted, and hence always have 
to
 | reload them after a function call /anyway/.

   That's why---it has to be safe, because it can't guarantee that it will 
only be called with const objects.

 | > In any case, I don't think it'll break any existing code if we were 
just
 | > to change it, so unless someone else can think of a problem, I'll 
schedule
 | > it for the next release.

   Good stuff.

   Even it broke code, we would still fix it, because the current situation 
is a bug, and we don't care about bug-compatibility; only feature- 
compatibility.

        /|
 o o o (_|/
        /|
       (_/



More information about the oslib-user mailing list