[portej05]

I really should have worded those much better than I did....

[portej05]

A new (very well specified) one for you all (and mostly from the engine ):

Code: [Select]

typedef struct color {
uint screen_sig;
ubyte red;
ubyte green;
ubyte blue;
ubyte alpha;
ubyte ac_type; // The type of alphacolor.  See AC_TYPE_??? defines
int is_alphacolor;
ubyte raw8;
int alphacolor;
int magic;
} color;

Here's the question:

Code: [Select]

sizeof( color ) == x

What is x, and why.

[Hery]

28 (at least on x86)
Data structure alignment requires address of variable to be a multiplicity of its size. That's why when we have, for example:

Code: [Select]

struct foo {
    char one;
    int two;
    char three;
};

struct bar {
    char one;
    char two;
    int three;
};
foo size is 12 bytes while bar takes 8 bytes of memory. If we really need data to be packed one directly after another then we can use implementation defined directives (that's useful while dealing with low-level stuff).
The whole data alignment stuff depends on the implementation and is not a thing we should rely on (unless using kind of packed attribute or using only x86).

[portej05]

OK, here's another (from line 342 of ddsutils.cpp):

Code: [Select]

char real_filename[MAX_FILENAME_LEN];
...
memset( &real_filename, 0, sizeof(real_filename) );

What's wrong with it, and would it work anyway?

[Uchuujinsan]

Code: [Select]

char real_filename[MAX_FILENAME_LEN];
...
memset( real_filename, 0, sizeof(char)*MAX_FILENAME_LEN );
That's the way it should work, shouldn't it?

Code: [Select]

memset( &real_filename, 0, sizeof(real_filename) );
Should be similar to

Code: [Select]

real_filename = 0;


[Hery]

Code: [Select]

char real_filename[MAX_FILENAME_LEN];
...
memset( &real_filename, 0, sizeof(real_filename) );

What's wrong with it, and would it work anyway?

I was cheating a bit, because I had to check few things in the debugger (and I'm still not sure about the whole thing, though).
Apparently, the reference operator is redundant but is is not an error. "real_filename" is a name of an array of char, which can be converted to a pointer. It can be implicit or explicit, using reference operator. That's why real_filename == &real_filename.

Uchuujinsan: sizeof(char) is redundant since it by definition equals 1.

[portej05]

I'm sticking this here to remind me to go back and do it properly when we get a better memory management solution

Line 1053 of modelread.cpp

Code: [Select]

for ( i = 0; i < pm->n_models; i++ )
{
  /* HACK: This is an almighty hack because it is late at night and I don't want to screw up a vm_free */
  new ( &( pm->submodel[ i ].buffer ) ) SCP_vector<buffer_data>( );
  pm->submodel[ i ].Reset( );
}


[Uchuujinsan]

Hery:
I know, thx, I just wrote it there to make the structure clearer. The compiler would remove it anyway, so it has no runtime influence.

[Spicious]

If it didn't automatically convert the array to a pointer it wouldn't compile. Arrays are like literals; they don't have addresses in that sense. real_filename = 0 would not compile for that reason. In terms of consistency it's still horrible though. Using sizeof(real_filename) is fine as long as it's an array and not a pointer.

[Uchuujinsan]

Hm, interesting, didn't know that :>

[Hery]

Spicious: Arrays are nonmodifiable lvalues (thus they have addresses) while literals are always rvalues.

[portej05]

Here's one from all over the engine (a simplified case):

Code: [Select]

struct SomeStruct
{
  int x;
  SomeStruct( )
       : x(10)
  {
  }
};

SomeStruct* ss = vm_malloc( sizeof(SomeStruct) );

What is the value of 'x' when accessed via ss->x after the call to vm_malloc, why, and if there is a problem, what code should be changed?

[Uchuujinsan]

I'm not entire sure what vm_malloc does, but I would guess it doesn't call the constructor, so ss->x should be 0 in debug mode, and unitialized/random in release.
Should be fixed by using "new" instead.
Maybe I guess right this time :>

[portej05]

Indeed, you are correct.
The value of x is actually undefined though in both release and debug modes
Visual Studio debug CRT will initialise it to 0xcdcdcdcd (ever wondered why malloc is so slow in debug modes!)
New is the solution to the problem.
You can either do placement new (i.e. new ( ss ) SomeStruct( ) ) or just replace the vm_malloc with new SomeStruct( );

[Uchuujinsan]

Ah ok, I've only used VS yet, and I knew that debug mode always initilizes to a fixed value while release does not.

[portej05]

OK, programming question time (from parselo.cpp):

What is the reason that you need const_cast?

Code: [Select]

bool can_construe_as_integer(const char *text)
{
// trivial case; evaluates to 0
if (*text == '\0')
return true;

// number sign or digit for first char
if ((*text != '+') && (*text != '-') && !isdigit(*text))
return false;

// check digits for rest
// (why on earth do we need a const cast here?  text isn't the pointer being modified!)
for (char *p = const_cast<char*>(text) + 1; *p != '\0'; p++)
{
if (!isdigit(*p))
return false;
}

return true;
}


[Aardwolf]

OK, programming question time (from parselo.cpp):

What is the reason that you need const_cast?

*snip*

The answer is obvious: code gremlins. If you don't const_cast, they will sabotage your code at runtime and cause you to crash.

[Uchuujinsan]

isdigit(char*) instead of isdigit(const char*)?

[Hery]

Uchuujinsan: isdigit is not interested in pointers, it wants values ;p

In this part of code we can easily get rid of const_cast by making p const char*. We still would be able to increment p value:
const char * p; <- pointer to constant data
char * const p; <- constant pointer to data

[Uchuujinsan]

ah, damn, didn't look close enough :>