Mike,<br><br>Yeap, all those "implicit" uses of "long" are bugs<br>and must be fixed.<br><br>All this now falls under the umbrella of Bug 9260.<br><br>Unfortunately there is no shortcut on how to fix these <br>
issues, we have to go one by one....<br><br>As you pointed out, right now we are focusing on fixing<br>Index, Offset and Size, but... any other uses of "long"<br>are suspect...<br><br>In retrospective, this is the result of being sloppy in our<br>
practices of Generic Programming, and allowing our<br>knowledge of the Index component being "long" to<br>percolate in the downstream code.<br><br>We should have been using Traits from Index, Size <br>and Offset all this time.<br>
<br><br><br> Luis<br><br><br>----------------------------------------------------------------------<br><div class="gmail_quote">On Mon, Jul 13, 2009 at 12:04 PM, Michael Jackson <span dir="ltr"><<a href="mailto:mike.jackson@bluequartz.net">mike.jackson@bluequartz.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
---<br>
Mike Jackson <a href="http://www.bluequartz.net" target="_blank">www.bluequartz.net</a><div class="im"><br>
<br>
<br>
<br>
On Jul 13, 2009, at 11:30 AM, Bradley Lowekamp wrote:<br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Luis,<br>
<br>
This looks like an absolutely massive effort you are attempting here. I have grepped through the iterator classes are there alot of incorrect usages of long:<br>
<br>
itkImageRegionReverseConstIterator.h: m_SpanEndOffset = this->m_BeginOffset - static_cast<long>(this->m_Region.GetSize()[0]);<br>
</blockquote>
<br></div>
So that is a "bug" and should be corrected. This effort will help get rid of those.. I hope. Turning on -Wall may help track those down a bit easier but with the size of the code base of ITK there is going to be a large effort to track those down.<div class="im">
<br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
And then there are complex relations like this:<br>
<br>
itkSize.h: typedef unsigned long SizeValueType;<br>
itkImageRegion.h: typedef Size< itkGetStaticConstMacro( ImageDimension ) > SizeType;<br>
itkImageRegion.h: typedef typename SizeType::SizeValueType SizeValueType;<br>
itkImageRegion.h: SizeValueType GetNumberOfPixels() const;<br>
<br>
<br>
I am not really sure how all these issues can be tracked down. From the user list:<br>
<br>
<br>
On Jul 10, 2009, at 12:46 PM, Michael Jackson wrote:<br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
IMNSHO, ITK (and VTK by extension) should absolutely BAN the use of<br>
"long" in their projects. Period. Too many avoidable bugs come up from<br>
its use. If you want a 32 bit integer use int or the standard ansi int<br>
type, if you want a 64 bit integer then use "long long int" or the<br>
standard ansi 64 bit integer type.<br>
"long int" is just a mess waiting to happen. Maybe a rule should be<br>
put into the KWStyle project to look for and flag the use of 'long'?<br>
</blockquote>
<br>
<br>
Do we need to go as far as banning the use of long and unsigned long?<br>
<br>
<br>
Good Luck Luis,<br>
Brad<br>
</blockquote>
<br></div>
I stated that without fully understanding what the SizeValueType really represented. For this case I think Luis has settled on using ptrdiff_t instead of "long". But I _still_ stand by my opinion that any use of "long" is bound to have really bad subtle bugs which are hard to find and fix.<br>
<br>
<br>
Mike Jackson<div><div></div><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
On Jul 10, 2009, at 6:33 PM, Luis Ibanez wrote:<br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
On recent realization:<br>
<br>
"size_t" is unsigned :-/<br>
<br>
and we need the OffsetValueType to be signed,<br>
since we use it to compute differences...<br>
<br>
It seems that what we need is the type<br>
<br>
"ptrdiff_t"<br>
<br>
which is supposed to represent the differences<br>
between two pointers, and therefore should be<br>
capable of measuring distances between any<br>
two locations in memory.<br>
<br>
I'm now rerunning the Experimental with<br>
"ptrdiff_t" instead of "size_t".<br>
<br>
<br>
Luis<br>
<br>
------------------------------------------------------------------------------------------------------<br>
On Fri, Jul 10, 2009 at 12:30 PM, Luis Ibanez <<a href="mailto:luis.ibanez@kitware.com" target="_blank">luis.ibanez@kitware.com</a>> wrote:<br>
Hi Kana,<br>
<br>
Thanks a lot for looking into this and sharing your findings.<br>
<br>
I just confirmed that in Windows 64bits, the "long" type is<br>
only 4 bytes.<br>
<br>
Here is the program I used:<br>
<br>
#include <iostream><br>
#include "itkOffset.h"<br>
#include "itkNumericTraits.h"<br>
<br>
<br>
int main()<br>
{<br>
unsigned long tt;<br>
std::cout << "size = " << sizeof(tt) << std::endl;<br>
tt = -1;<br>
std::cout << "tt = " << tt << std::endl;<br>
<br>
typedef itk::Offset<3> OffsetType;<br>
typedef OffsetType::OffsetValueType OffsetValueType;<br>
<br>
OffsetValueType offsetValue;<br>
<br>
std::cout << "sizeof(offsetValue) = " << sizeof( offsetValue ) << std::endl;<br>
<br>
offsetValue = itk::NumericTraits< OffsetValueType >::max();<br>
<br>
std::cout << "OffsetValueType max() = " << offsetValue << std::endl;<br>
<br>
return EXIT_SUCCESS;<br>
}<br>
<br>
<br>
with this CMakeLists.txt file<br>
<br>
<br>
CMAKE_MINIMUM_REQUIRED(VERSION 2.4)<br>
IF(COMMAND CMAKE_POLICY)<br>
CMAKE_POLICY(SET CMP0003 NEW)<br>
ENDIF(COMMAND CMAKE_POLICY)<br>
<br>
<br>
PROJECT(64bitsTest)<br>
<br>
FIND_PACKAGE(ITK REQUIRED)<br>
INCLUDE(${ITK_USE_FILE})<br>
<br>
ADD_EXECUTABLE(typesTest typesTest.cxx )<br>
<br>
TARGET_LINK_LIBRARIES(typesTest ITKCommon)<br>
<br>
<br>
<ATT00001.txt><br>
</blockquote>
<br>
========================================================<br>
Bradley Lowekamp<br>
Lockheed Martin Contractor for<br>
Office of High Performance Computing and Communications<br>
National Library of Medicine<br>
<a href="mailto:blowekamp@mail.nih.gov" target="_blank">blowekamp@mail.nih.gov</a><br>
</blockquote>
<br></div></div><div><div></div><div class="h5">
_____________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at<br>
<a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Please keep messages on-topic and check the ITK FAQ at: <a href="http://www.itk.org/Wiki/ITK_FAQ" target="_blank">http://www.itk.org/Wiki/ITK_FAQ</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.itk.org/mailman/listinfo/insight-users" target="_blank">http://www.itk.org/mailman/listinfo/insight-users</a><br>
</div></div></blockquote></div><br>