incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: [jira] Commented: (STDCXX-761) [HP aCC 6.16] Out of bound access in new.cpp
Date Fri, 04 Apr 2008 00:19:33 GMT
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?

Btw., C99 added this _Pragma() operator. It can be used
just like #pragma except it's not a directive so it can
be the result of preprocessor expansion. aCC seems to
grok it, so instead of adding a pair of

     #if 60000 <= __HP_aCC
     #  pragma diag_suppress 20206
     #endif

and

     #if 60000 <= __HP_aCC
     #  pragma diag_default
     #endif

around tricky code all over the place we could have

     #if 60000 <= __HP_aCC
     #  define _RWSTD_aCC_PRAGMA(text)     _Pragma (text)
     #else
     #  define _RWSTD_aCC_PRAGMA(ignore)   (void)0
     #endif

and simplify the tricky code guards to:

     _RWSTD_aCC_PRAGMA ("diag_suppress 20206")
     /* tricky code */
     _RWSTD_aCC_PRAGMA ("diag_default")

Martin

> 
> 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 [] =
>     {
>         &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


Mime
View raw message