Martin Sebor wrote:
> Travis Vitek wrote:
>>  
>>
>>> Martin Sebor wrote:
>>>
>>> Eric Lemings wrote:
>>>>  
>>>> Travis,
>>>>
>>>> If you could, give the following patch a whirl (or quick review
>>>> at least).
>>> Is there an easier way to silence the warning than by adding
>>> all these loops? E.g., by assigning the address of the first
>>> member to a local pointer? (Or is the compiler too smart for
>>> that?)
>>
>> That is exactly what the previous code did...
>>
>>     size_t*       diffs    = diff.new_calls_;
>>     const size_t* st0_args = st0->new_calls_;
>>     const size_t* st1_args = st1->new_calls_;
>>
>> The compiler complains because the member new_calls_ is an array of
>> length 2, but we iterate over 16 elements. Provided that there is no
>> padding between each of the array fields, this would work and we'd just
>> have to disable the warning.
> 
> Wouldn't that be the way to go then? I.e., wrap the loop
> in a couple of #pragma diag_suppress/diag_default?

On second thought, I have to agree that the code is tricky and brittle.
If someone were to change the order of the rwt_free_store members or
add a new one in between the loop would silently break. So maybe the
right solution is as Brad proposed...

> 
> Btw., C99 added this _Pragma() operator.
[...]
>> I don't really like the macro, probably because it uses local variables
>> without taking them as parameters. Another option would be to use offset
>> pointer to member and a nested loop.
>>
>>     typedef size_t (rwt_free_store::* checkpoint_field)[2];
>>     static const checkpoint_field fields [] =
>>     {

...or this, except, of course, for the braces which in local/class
scope go on the line above ;-)

You guys decide, either way is fine with me.

Btw., if you go with Brad's approach, there's no need for EXTENT().
We already have an RW_COUNTOF() macro in <testdefs.h>.

Oh, and another last thing: new.cpp uses a naming convention that's
inconsistent with the rest of the driver where non-member functions
with internal linkage (i.e., namespace-scope statics) start with
the _rw_ prefix, and extern functions with rw_. At some point we
need to change the externs but any new statics should use the
_rw_ convention.

Martin

>>         &rwt_free_store::new_calls_,
>>         &rwt_free_store::delete_calls_,
>>         &rwt_free_store::delete_0_calls_,
>>         &rwt_free_store::blocks_,
>>         &rwt_free_store::bytes_,
>>         &rwt_free_store::max_blocks_,
>>         &rwt_free_store::max_bytes_,
>>         &rwt_free_store::max_block_size_
>>     };
>>
>>     static rwt_free_store diff;
>>
>>     bool diff_0 = true;
>>
>>     for (size_t f = 0; f < sizeof (fields) / sizeof (*fields); ++f)
>>     {
>>         size_t*       diffs    = (diff.*fields [f]);
>>         const size_t* st0_args = (st1->*fields [f]);
>>         const size_t* st1_args = (st0->*fields [f]);
>>
>>         for (size_t i = 0; i < 2; ++i)
>>         {
>>           diffs [i] = st1_args [i] - st0_args [i];
>>           if (diffs [i])
>>             diff_0 = false;
>>         }
>>     }
>>
>>
>>> Also, there is no need to use the _RWSTD_SIZE_T macro in .cpp
>>> files. The macro is useful in library and test suite headers
>>> to reduce namespace pollution (so we don't have to #include
>>> <stddef.h>.
>>
>> I was going to mention that.
>>
>>> Last thing: the code formatting convention calls for a space
>>> before each open parenthesis. It might take some getting used
>>> to but once you do, you'll never go back -- just ask Travis ;-)
>>>
>>
>> Uh, yeah.
>>
>> Travis
>