the function copy() of OFString returns wrong length

[Chung-Yueh Lien]

Hi, I found a problem while using the function copy(). The test code is shown as following:

Code: Select all

 OFString Req = "PatientID=AV35674&ScheduledProcedureStepSequence.ScheduledProcedureStepStartDate=20180508";
  char *str = new char[Req.size()];
  Req.copy(str, Req.size());

  COUT << "size:" << Req.size() << "\r\n";
  COUT << "length:" << Req.length() << "\r\n";
  COUT << "strlen:" << strlen(str) << "\r\n";
  COUT << "c_str:" << Req.c_str() << "\r\n";
  COUT << "data:" << Req.data() << "\r\n";
  COUT << "copy:" << str << "\r\n";

The result shows

Code: Select all

size:89
length:89
strlen:104
c_str:PatientID=AV35674&ScheduledProcedureStepSequence.ScheduledProcedureStepSta
rtDate=20180508
data:PatientID=AV35674&ScheduledProcedureStepSequence.ScheduledProcedureStepStar
tDate=20180508
copy:PatientID=AV35674&ScheduledProcedureStepSequence.ScheduledProcedureStepStar
tDate=20180508垂垂垂垂

The return string of the copy() is longer than using str()/data() that is not correct.

PS: the HAVE_MEMCPY is defined.

[J. Riesmeier]

Hi Chung-Yueh,

could it be that simple that your char array is one byte too short? OFString::size() returns the length of the contained character string, so you have to add 1 when allocating the array (for the terminating 0 byte).

[Chung-Yueh Lien]

The result is the same.

could it be that simple that your char array is one byte too short?

BTW, the dcmtk-3.6.1_20121102 does not have this problem.

Also I tested strncpy() and strcpy to copy string from OFString.

Code: Select all

OFString Req = "PatientID=AV35674&ScheduledProcedureStepSequence.ScheduledProcedureStepStartDate=20180508";
	char *str = new char[Req.size()];
	char *str1 = new char[Req.size()];
	//Req.copy(str, Req.size());
	strncpy(str, Req.data(), Req.size());
	strcpy(str1, Req.data());

	COUT << "str:" << str << "\r\n";
	COUT << "str.len:" << strlen(str) << "\r\n";
	COUT << "str1:" << str1 << "\r\n";
	COUT << "str1.len:" << strlen(str1) << "\r\n";

the result shows that the strcpy is correct.

Code: Select all

str:PatientID=AV35674&ScheduledProcedureStepSequence.ScheduledProcedureStepStartDate=20180508垂垂垂垂
str.len:104
str1:PatientID=AV35674&ScheduledProcedureStepSequence.ScheduledProcedureStepStartDate=20180508
str1.len:89

I also used memcpy(), and the result is the same as copy() and strncpy().

[J. Riesmeier]

I checked this more thoroughly: OFString::copy() is consistent with string::copy() by "not adding a null character at the end of the copied content".
Also strncpy() does not add the terminating null character. That's why we use our own OFStandard::srtrlcpy() and OFStandard::strlcat() in the DCMTK

[Chung-Yueh Lien]

Hi Jörg,

I assigned a null character at the end of position, code like this

Code: Select all

char *str = new char[Req.size()+1];
Req.copy(str, Req.size());
str[Req.size()+1]='\0';

However, I hope OFString::copy() could solve this problem as well.

Thank you for your reply.

[J. Riesmeier]

As far as I can see, there is nothing to be solved with the implementation of OFString::copy(). As I wrote, this is how string::copy() is defined.

The question is: why do you use the copy() method at all?

[Chung-Yueh Lien]

The question is: why do you use the copy() method at all?

I usually write an C interface to wrap native DLL for calling by other program language such as C/C++, C#, VB. When I design the string arguments to DLL function, the data type of OFString will be transferred to char* allowing other program language to call the DLL.

The other reason is v3.6.1 without this problem. So, if I upgrade to 3.6.3, I must modified all OFString::copy() I used in source code.

[J. Riesmeier]

When I design the string arguments to DLL function, the data type of OFString will be transferred to char* allowing other program language to call the DLL.

Then, you should/could use the OFString::c_str() method.

The other reason is v3.6.1 without this problem. So, if I upgrade to 3.6.3, I must modified all OFString::copy() I used in source code.

Understood. But OFString::copy() was buggy (and not standard-conformant). This has been fixed about 5 years ago: http://git.dcmtk.org/?p=dcmtk.git;a=com ... 818697349f

[Jan Schlamelcher]

Also, in case you did not know, you can replace the usage of OFString with std::string in the entire toolkit using the CMake Cache variable(s) DCMTK_ENABLE_STL / DCMTK_ENABLE_STL_STRING. Depending on what other program language you are working with (i.e. whether it has an automatic wrapper for std::string) this might also help regarding the issue.