sizeof (RopeRefCountString)

Hi,

In RewriteRope.cpp, RewriteRope::MakeRopeString(const char *Start, const char *End),
we calculate the AllocSize by:

unsigned AllocSize = sizeof(RopeRefCountString) - 1 + AllocChunkSize;

I guess here the intention is: sizeof(RopeRefCountString) is 5. But gcc says sizeof(RopeRefCountString) is 8. So the actual AllocSize is 4087. Should we minus 4 instead of 1 to make the AllocSize 4084?

-Zhongxing Xu

The "-1" I believe is to accommodate for the field Data[1], which occupies a single byte:

   struct RopeRefCountString {
     unsigned RefCount;
     char Data[1]; // Variable sized.

     void addRef() { ... }
     void dropRef() { ... }
   };

By subtracting 1, the field "Data" refers to an array with size AllocChunkSize.

That was the intention, but it forgot the tail padding, so this is a real 'bug'. The intention was the make the allocation just under a page in size. Is this causing a problem in practice?

-Chris

2008/9/16 Chris Lattner <clattner@apple.com>

2008/9/16 Ted Kremenek <kremenek@apple.com>

Right. Isn’t there a pointer arithmetic trick we can do here to get the right number?

"offsetof(RopeRefCountString, Data)"?

-Eli

#include <stddef.h>

unsigned AllocSize = sizeof(RopeRefCountString) -
offsetof(RopeRefCountString,Data) + AllocChunkSize;

Thanks Eli.

Using offsetof, probably the number we want is:

   offsetof(RopeRefCountString, Data) + AllocChunkSize

I don't think that's right. sizeof(RopeRefCountString) - offsetof(RopeRefCountString,Data) is 4. What we want is the number of bytes before "data", not after (inclusive).

Probably:

   offsetof(RopeRefCountString,Data) + AllocChunkSize

May I commit the patch:

Index: lib/Rewrite/RewriteRope.cpp

That looks good to me unless anyone has any objections. I assume it passes “make test.”