Re: svn commit: r630244 - /stdcxx/trunk/src/time_put.cpp

[Martin Sebor <se...@roguewave.com>]

faridz@apache.org wrote:
> Author: faridz
> Date: Fri Feb 22 08:34:52 2008
> New Revision: 630244
> 
> URL: http://svn.apache.org/viewvc?rev=630244&view=rev
> Log:
> 2008-02-22  Farid Zaripov  <fa...@epam.com>
> 
> 	* src/time_put.cpp (__rw_get_timepunct): Remove const qualifier to disable
> 	eccp warning #191-D: "type qualifier is meaningless on cast type".

I was looking at this code wondering if the const was there for
a reason (maybe to get around a compiler bug) but I think it got
introduced simply by accident.

While poring through it though, I noticed a reinterpret cast in
a call to operator delete() further down that was introduced in
the original patch:
   http://svn.apache.org/viewvc?view=rev&revision=587215
I don't think that cast is necessary anymore, since we're not
invoking the delete expression but calling operator delete
directly (and it doesn't call the dtor on the object).

I think we can safely remove the cast. Here's the patch I plan
to commit unless someone speaks up against it in the next couple
of hours:

Index: src/time_put.cpp
===================================================================
--- src/time_put.cpp    (revision 630253)
+++ src/time_put.cpp    (working copy)
@@ -615,7 +615,7 @@
                  _RWSTD_STATIC_CAST(char*, ::operator new (tmpsize));
              memcpy (tmp, pun, sizeof *pun + off);

-            ::operator delete (_RWSTD_REINTERPRET_CAST (char*, pun));
+            ::operator delete (pun);

              pun      = _RWSTD_REINTERPRET_CAST (__rw_time_t*, tmp);
              pmem     = _RWSTD_REINTERPRET_CAST (_RWSTD_UINT32_T*, pun);

Martin

> 
> Modified:
>     stdcxx/trunk/src/time_put.cpp
> 
> Modified: stdcxx/trunk/src/time_put.cpp
> URL: http://svn.apache.org/viewvc/stdcxx/trunk/src/time_put.cpp?rev=630244&r1=630243&r2=630244&view=diff
> ==============================================================================
> --- stdcxx/trunk/src/time_put.cpp (original)
> +++ stdcxx/trunk/src/time_put.cpp Fri Feb 22 08:34:52 2008
> @@ -612,7 +612,7 @@
>              // reallocate, again using operator new to avoid mismatch
>              // with facet destructor
>              char* const tmp = 
> -                _RWSTD_STATIC_CAST(char* const, ::operator new (tmpsize));
> +                _RWSTD_STATIC_CAST(char*, ::operator new (tmpsize));
>              memcpy (tmp, pun, sizeof *pun + off);
>  
>              ::operator delete (_RWSTD_REINTERPRET_CAST (char*, pun));
> 
> 

RE: svn commit: r630244 - /stdcxx/trunk/src/time_put.cpp

[Travis Vitek <Tr...@roguewave.com>]

 

>Martin Sebor wrote:
>
>While poring through it though, I noticed a reinterpret cast in
>a call to operator delete() further down that was introduced in
>the original patch:
>   http://svn.apache.org/viewvc?view=rev&revision=587215
>I don't think that cast is necessary anymore, since we're not
>invoking the delete expression but calling operator delete
>directly (and it doesn't call the dtor on the object).
>
>I think we can safely remove the cast. Here's the patch I plan
>to commit unless someone speaks up against it in the next couple
>of hours:
>
>Index: src/time_put.cpp
>===================================================================
>--- src/time_put.cpp    (revision 630253)
>+++ src/time_put.cpp    (working copy)
>@@ -615,7 +615,7 @@
>                  _RWSTD_STATIC_CAST(char*, ::operator new (tmpsize));
>              memcpy (tmp, pun, sizeof *pun + off);
>
>-            ::operator delete (_RWSTD_REINTERPRET_CAST (char*, pun));
>+            ::operator delete (pun);
>
>              pun      = _RWSTD_REINTERPRET_CAST (__rw_time_t*, tmp);
>              pmem     = _RWSTD_REINTERPRET_CAST 
>(_RWSTD_UINT32_T*, pun);
>
>Martin
>

Yup, the new guy messed up again. I can't imagine why I left the cast, I
know that it isn't necessary.

Your patch looks good though.