Skip to content

Commit

Permalink
Fix the way that MINC IO handles null-termination.
Browse files Browse the repository at this point in the history
When vtkMINCImageReader creates a vtkCharArray to store a NetCDF
attibute, it allocates one more byte than the number of chars, in
order to make room for a null terminator.  The way it did so was
unsafe and incompatible with the new vtkGenericDataArray changes.
This new code does the same thing, but in a compatible manner.
  • Loading branch information
dgobbi committed Apr 26, 2016
1 parent 9e0eb98 commit 329ddf6
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 41 deletions.
80 changes: 46 additions & 34 deletions IO/MINC/vtkMINCImageAttributes.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -298,26 +298,33 @@ void vtkMINCImageAttributes::AddDimension(const char *dimension,
const char *vtkMINCImageAttributes::ConvertDataArrayToString(
vtkDataArray *array)
{
const char *result = 0;
const char *result = "";
vtkIdType n = array->GetNumberOfTuples();
if (n == 0)
{
return result;
}

int dataType = array->GetDataType();

if (dataType == VTK_CHAR)
{
vtkCharArray *charArray = vtkCharArray::SafeDownCast(array);
result = charArray->GetPointer(0);
if (!result)
if (charArray)
{
result = "";
result = charArray->GetPointer(0);
// Check to see if string has a terminal null (the null might be
// part of the attribute, or stored in the following byte)
if ((n > 0 && result[n-1] == '\0') ||
(charArray->GetSize() > n && result[n] == '\0'))
{
return result;
}
}
return result;
}

std::ostringstream os;

vtkIdType n = array->GetNumberOfTuples();
vtkIdType i = 0;
for (i = 0; i < n; i++)
for (vtkIdType i = 0; i < n; i++)
{
double val = array->GetComponent(i, 0);
if (dataType == VTK_DOUBLE || dataType == VTK_FLOAT)
Expand All @@ -344,42 +351,47 @@ const char *vtkMINCImageAttributes::ConvertDataArrayToString(
}
os << storage;
}
else if (dataType == VTK_CHAR)
{
os.put(static_cast<char>(val));
}
else
{
os << val;
}
if (i < n-1)
if (i < n-1 && dataType != VTK_CHAR)
{
os << ", ";
}
}

// Store the string
std::string str = os.str();
// Store the string
std::string str = os.str();

if (this->StringStore == 0)
{
this->StringStore = vtkStringArray::New();
}
if (this->StringStore == 0)
{
this->StringStore = vtkStringArray::New();
}

// See if the string is already stored
n = this->StringStore->GetNumberOfValues();
for (i = 0; i < n; i++)
{
result = this->StringStore->GetValue(i);
if (strcmp(str.c_str(), result) == 0)
{
break;
}
}
// If not, add it to the array.
if (i == n)
// See if the string is already stored
vtkIdType m = this->StringStore->GetNumberOfValues();
vtkIdType j;
for (j = 0; j < m; j++)
{
result = this->StringStore->GetValue(j);
if (strcmp(str.c_str(), result) == 0)
{
i = this->StringStore->InsertNextValue(str.c_str());
result = this->StringStore->GetValue(i);
break;
}
}
// If not, add it to the array.
if (j == m)
{
j = this->StringStore->InsertNextValue(str.c_str());
result = this->StringStore->GetValue(j);
}

return result;
return result;
}

//-------------------------------------------------------------------------
Expand Down Expand Up @@ -888,11 +900,11 @@ void vtkMINCImageAttributes::SetAttributeValueAsString(
size_t length = strlen(value);

vtkCharArray *array = vtkCharArray::New();
array->SetNumberOfValues(length+1);
char *dest = array->GetPointer(0);
// Allocate an extra byte to store a null terminator.
array->Resize(length + 1);
char *dest = array->WritePointer(0, length);
strncpy(dest, value, length);
dest[length] = '\0';

this->SetAttributeValueAsArray(variable, attribute, array);

array->Delete();
Expand Down
9 changes: 4 additions & 5 deletions IO/MINC/vtkMINCImageReader.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -481,11 +481,10 @@ int vtkMINCImageReader::ReadMINCFileAttributes()
vtkCharArray *charArray = vtkCharArray::New();
// The netcdf standard doesn't enforce null-termination
// of string attributes, so we add a null here.
charArray->SetNumberOfValues(attlength+1);
charArray->SetValue(attlength, 0);
charArray->SetNumberOfValues(attlength);
nc_get_att_text(ncid, varid, attname,
charArray->GetPointer(0));
charArray->Resize(attlength + 1);
char *dest = charArray->WritePointer(0, attlength);
nc_get_att_text(ncid, varid, attname, dest);
dest[attlength] = '\0';
dataArray = charArray;
}
break;
Expand Down
8 changes: 6 additions & 2 deletions IO/MINC/vtkMINCImageWriter.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -498,11 +498,15 @@ nc_type vtkMINCImageWriterConvertVTKTypeToMINCType(
}

//-------------------------------------------------------------------------
// These macro is only for use in WriteMINCFileAttributes
// These macros are only for use in WriteMINCFileAttributes.

// Note: Until VTK 7.0, this macro added a terminating null byte to all
// text attributes. As of VTK 7.1, it does not. The attribute length
// should be the string length, not the string length "plus one".
#define vtkMINCImageWriterPutAttributeTextMacro(name, text) \
if (status == NC_NOERR) \
{ \
status = nc_put_att_text(ncid, varid, name, strlen(text)+1, text); \
status = nc_put_att_text(ncid, varid, name, strlen(text), text); \
}

#define vtkMINCImageWriterPutAttributeDoubleMacro(name, count, ptr) \
Expand Down

0 comments on commit 329ddf6

Please sign in to comment.